Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd mutator unittests #31
Merged
Conversation
|
I think this PR can be moved from draft status as this the dependent PR were merged |
|
The mutators that this tests are only in PR#26, which you just closed. |
|
I merged PR #26. but there are some conflicts now. can you please fix those? thx! |
The mutators weren't being tested so we only had to assume that they were doing the right thing. In my own tests I saw a lot of input that had NUL bytes in it, so I'm pretty sure that some of the mutations were not doing the right thing. With these tests, the copy method was found to have poorly named parameters. The pattern it's using is copy(a, b, posa, posb, lena, lenb) (where lena and lenb can be omitted). The 'a' parameter is where the update will take place, and the 'b' parameter is where the copy originates. However, the first parameter was called 'src' and the second parameter 'dst', which is the precise opposite of the expectation. The naming of the parameters was kept consistent (a named as src, despite being the destination) with the other named parameters and within the function. This has been corrected, and with the correct naming, it became obvious that the insert, remove and duplicate functions were not working as intended. It is unclear what the difference is intended to be for Duplicate and Copy bytes - I have 'fixed' Duplicate, but this means that it now works identically to Copy, so it's not clear to me what's meant to be done there.
Now that we have unit tests, these can be invoked by the Makefile.
The test files have now been updated to include annotations, in the file prologue comment, which describe the test and its place in the testing environment. These are just a convention that I've used previously, but they help to focus anyone doing testing on describing where they fit into the testing of the system. Such descriptions make it obvious when reviewed where there are gaps in testing.
|
Done, making this quite a small PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
gerph commentedJan 12, 2020
•
edited
This PR requires PR #26,. #27, #28, #29 and #30 to be merged first.
This change adds unit tests for some of the mutators, and addresses some issues that were found when the mutators were being tested.
The mutators are tested with the 'unittest' level of testing, meaning that they only invoke the unit of the module, and should not be affected by anything outside that module. As such they're very limited. Although they test only a couple of cases for each mutator they have found problems because the operation they were expected to perform hasn't worked.
This PR comprises 3 changes:
The latter of these could be independant, or dropped; it has been useful to me.