Extend MembersThat and MembersShould to have starting,… (Resolves #239)#314
Conversation
|
👉 View analysis in DeepCode’s Dashboard |
|
Cool, thanks! 😉 |
| CONJUNCTION haveFullNameNotMatching(String regex); | ||
|
|
||
| /** | ||
| * Asserts that members have a full name starting with given prefix. |
There was a problem hiding this comment.
If I'm not mistaken, the new methods test member names, but not full names (which would include their owners' FQCN).
| $(described(methods().that().haveNameEndingWith("C")), ImmutableSet.of(METHOD_C)), | ||
| $(described(constructors().that().haveNameEndingWith("it>")), ALL_CONSTRUCTOR_DESCRIPTIONS), | ||
| $(described(fields().that().haveNameEndingWith("B")), ImmutableSet.of(FIELD_B)), | ||
|
|
There was a problem hiding this comment.
We could probably add similar test coverage for the members should functionality in MembersShouldTest.
|
Update:
|
codecholeric
left a comment
There was a problem hiding this comment.
Thanks a lot for your contribution, I really appreciate it! 😃 Sorry that it took me so long to look into it, my pipeline was unfortunately full 😞
I added two minor review commits and one bigger commit where I added the negated forms. I've noticed I forgot to add those to the issue, but to be consistent I think it makes sense.
Can you look at my commits and tell me if that looks okay to you?
If yes I would suggest you squash your commits and my little review commits into one commit (containing the positive additions to the syntax) and then keep my last commit (adding the negated version) as a separate commit on top. That way we have semantically relevant commits for the master commit history 😉
codecholeric
left a comment
There was a problem hiding this comment.
Also: Can you double check your signed-off part? It seems a little weird, your signature is always
Signed-off-by: kamer <kamer@kamerelciyar.com>
But your author name in the commits changes from Kamer Elciyar to kamer which the DCO check doesn't like I guess 😉 But when you squash your commits it might be easy to fix.
If this is all not completely clear to you, simply ask! 😃
|
One thing I noticed though (out of scope for this PR) is that we really should look into this zoo of |
|
@kamer Any update? |
|
@codecholeric I updated like you have described and build with |
… ending functionality. (#239) Signed-off-by: Kamer Elciyar <kamer@kamerelciyar.com>
I have also added some tests for the failure details, since broken messages there would be undetected at the moment. The reason that some of these tests for members are missing for other methods is simply that we reuse those for classes, so they were not added in the past since those tests would have already caught it. If we have syntax methods explicitly for members (that are not also used for classes), then we should add a detailed test checking that the failure details indeed match. when I wrote those tests I also noticed that messages like `Constructor<very.long.fqn.<init>()> does not start with 'foo'` does also read slightly weird (because there is no focus on the "name" the "starts with" refers to) so I added "name" to the failure details, i.e. `Constructor<..> name does not start with...`. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
|
@kamer thanks for cleaning it up!! 😃 When I tested the branch locally the tests also didn't pass, because of changed logic with respect to the line number -> #344 I adjusted the commit message of your commit and removed all the blubber from my review commits, because I do not think these comments will provide much benefit (adjust javadoc, etc.), when I read this history entry in 6 months 😉 (you can adjust the message, when you squash commits, you don't have to append them all to one long sequence) Anyway, thank you so much for your support!! Gonna merge it now 😃 |
… containing and ending functionality.
It's my first attempt to contribute an open-source project.
Hope this solves the issue properly.
Resolves: #239