Skip to content

Use more specific AssertJ MapAssert for return of JsonContentAssert#extractingJsonPathMapValue #9675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Use more specific AssertJ MapAssert for return of JsonContentAssert#extractingJsonPathMapValue #9675

wants to merge 3 commits into from

Conversation

@mihhail-lapushkin
Copy link
Contributor

@mihhail-lapushkin mihhail-lapushkin commented Jul 4, 2017 •
edited
Loading

Fixed this: #9650
[ sorry for typo in the commit message :( ]

@pivotal-issuemaster
Copy link

pivotal-issuemaster commented Jul 4, 2017

@mihhail-lapushkin Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

pivotal-issuemaster commented Jul 4, 2017

@mihhail-lapushkin Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 4, 2017
@philwebb philwebb mentioned this pull request Jul 5, 2017
Closed
@philwebb
Copy link
Member

philwebb commented Jul 5, 2017 •
edited by wilkinsona
Loading

Thanks for the PR. No worries about the commit message, we'll fix that when we merge.

@philwebb philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 5, 2017
@philwebb philwebb added this to the 1.5.5 milestone Jul 5, 2017
@filiphr
Copy link
Contributor

filiphr commented Jul 5, 2017

@philwebb I don't know your policy on this things, but I would propose that you align the returns of AssertJ with the returns of the JsonContentAssert. The same problem happens for JsonContentAssert#extractingJsonPathArrayValue. Sorry that I didn't mention it in the initial ticket, I could open another one if you want to.

@philwebb
Copy link
Member

philwebb commented Jul 5, 2017 •
edited by wilkinsona
Loading

Thanks @filiphr, no need for a new ticket as it's all related to the same underlying issue.

@mihhail-lapushkin if you're able to update the PR with the addition methods, that would be fantastic. Otherwise we'll do it when we merge.

@philwebb philwebb added the for: merge-with-amendments Needs some changes when we merge label Jul 5, 2017
@mihhail-lapushkin
Copy link
Contributor Author

mihhail-lapushkin commented Jul 6, 2017

Fixed JsonContentAssert#extractingJsonPathArrayValue. Not sure if others need fixing / could be fixed.

@filiphr
Copy link
Contributor

filiphr commented Jul 7, 2017

I think that the others are OK.

Theoretically what could be done if it is needed on spring boot side is to have a test that will check the signatures between AssertJ and JsonContentAssert, but that might be an overkill...

@wilkinsona wilkinsona self-assigned this Jul 21, 2017
wilkinsona pushed a commit that referenced this pull request Jul 21, 2017
@mihhail-lapushkin @wilkinsona
Fix JsonContentAssert type safety warnings
ae91c84 
Previously, JsonContentAssert returns AbstractMapAssert from
extractingJsonPathMapValue. This could lead to type safety warnings
when calling one of the assert's methods with a generic varargs
parameter such as
contains(Entry<? extends Object, ? extends Object>...).

This commit replaces the use of both AbstractMapAssert and
AbstractListAssert with MapAssert and ListAssert respectively. These
classes use final methods and @SafeVargs args to prevent the
above-described problem from occurring.

See gh-9675
wilkinsona added a commit that referenced this pull request Jul 21, 2017
@wilkinsona
Merge pull request #9675 from Mihhail Lapushkin
a57a741 
* gh-9675:
  Polish "Fix JsonContentAssert type safety warnings"
  Fix JsonContentAssert type safety warnings
@wilkinsona
Copy link
Member

wilkinsona commented Jul 21, 2017

Thanks very much for the PR, @mihhail-lapushkin. I've merged it into 1.5.x and master with some minor polishing.

wilkinsona added a commit that referenced this pull request Jul 21, 2017
@wilkinsona
Polish javadoc in JsonContentAssert
ece66e1 
See gh-9675
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@wilkinsona wilkinsona

Labels

for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement

Projects

None yet

Milestone

1.5.5

Development

Successfully merging this pull request may close these issues.

6 participants

@mihhail-lapushkin @pivotal-issuemaster @philwebb @filiphr @wilkinsona @spring-projects-issues