Issue #13809: Kill mutation in Tree-Walker-2#13976
Conversation
70e7baa to
a567b88
Compare
5e0c2d4 to
82b3283
Compare
82b3283 to
556acfb
Compare
|
There was some problem with GitHub, it didn't show in PR my latest push, so I created new PR, looks like now all is good. |
|
@nrmancuso @romani It looks like URL got "Check no closed issue references" is being generated wrong. |
| * This test is required for pitest coverage. | ||
| * Order of Check execution is not visible from outside | ||
| * as we order violations by line and column. | ||
| * This test is checking that Checks execution ordered by name and then by id. |
There was a problem hiding this comment.
Order of Check execution is not visible from outside
This is somewhat incorrect. This code came from random ordering of Javadoc checks. Multiple checks printed the same violation, but different executions picked which check was being printed as the source of the violation. This would cause differences in difference reports as we display who is the source of the violation.
The specific code from pitesting shows, we order Checks by class name, id, and then by hashcode. Those are the ordering of the execution of checks. Order of violations is defined elsewhere.
ordered by name and then by id.
Show how this test ensures checks are ordered by id too? I only see somewhat reliance on check name ordering.
There was a problem hiding this comment.
The whole javadoc should just be:
This test is checking that Checks execution ordered by name.
| final Checker checker = createChecker(treeWalkerConfig); | ||
|
|
||
| try { | ||
| checker.process(files); |
There was a problem hiding this comment.
Why can't this be a BDD test? I am not seeing anything really preventing it.
There was a problem hiding this comment.
All tries to make it in bad style did not kill mutation. Because mutation on order of checks, order of checks is not publicly visible.
I tried bunch and gave up.
There was a problem hiding this comment.
Have we tried something like the following?
- config with two checks
- both checks have bad config (maybe property name)
- we assert that it is the first check in the config that reported in the exception
?
There was a problem hiding this comment.
I am not , but exception will happen before Check is added to sorted-set of Checks of Treewker.
Order of check to load from config is defined by xml, predictable order, but mutation is for order of execution in validation runtime (traverse of tree).
I tried to keep test clear that first execution on beginTree will throw exception, so it very clear definition that Checks are ordered.
After bunch of time with this mutation, I am very ok to keep it as non bdd, because final variant of test is still easy to understand, I hope.
There was a problem hiding this comment.
Yes, test is easy to understand.
nrmancuso
left a comment
There was a problem hiding this comment.
A hack for sure, but let's not spend too much time on this.
Issue #13809: Kill mutation in Tree-Walker-2