Skip to content
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.

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

Mutation Testing ( splitted into 2 PRs ) #4089

Closed

Conversation

ASuciuX
Copy link
Contributor

@ASuciuX ASuciuX commented Nov 24, 2023

Description

Mutation Testing is a technique of evaluating the effectiveness of a series of tests by introducing small changes to the code (mutations) and checking if the tests can detect these small changes.

They are useful because they can show which functions are not tested properly and tell that their tests are missing, timing out or simply those functions are unviable to be tested.

In this PR 2 mutation testing implementations are provided:

  1. filter-pr which has the role on the **PR** to run the mutants for the functions highlighted by the git diff ( functions modified or created ).
  • If the functions don't have missing, timeout or unviable tests, the workflow will pass.
  • Otherwise it will fail and display which are the missing, timeout and unviable functions. In the case that a specific function shouldn't be tested, it can be highlighted so by specifying in its header #[mutants::skip] or other attributes containing mutants::skip (e.g. #[cfg_attr(test, mutants::skip)]).
  1. logger which has the role of keeping track of all the functions that weren't tested before implementing this. Its final role is to be removed by having complete completion, but till that point, it makes it a lot easier to see which functions need extra tests and add them. On **merge**, it will run cargo mutants in the same way as the filter-pr and will modify the logged output with the information from the new generated output. (eg. fn A had missing tests before, now it has the tests. it will be removed from missed.txt and appended to caught.txt)

Applicable issues

Additional info (benefits, drawbacks, caveats)

Solutions and Recommended Usage

Cargo-mutant does an incremental build for each mutation, that’s why this solution comes with the implementation of tracking the output and only updating it with the differences, to run 10-20 mutants instead of 4000.

CI YML File: Are linked with the actions in the actions' repo stacks-network/actions#3.

Should be executed with every PR, as it tests only the updated or newly added functions in the PR's commits.

How to read the status from the filter-pr workflow

If it is not failing, it means all the functions are tested properly.
If it is failing it will display what type of tests are the problem: missing tests, timeout tests, or unviable tests. For each section, it will specify the functions, the files they are in and their line numbers.

Handling Exit Codes (docs)

Mutation testing produces exit codes post-completion.
In the filter-pr workflow they are caught and specified with extra details about what should be done based on them.
In the logger workflow they are updating the place they belong to.

Benchmarks

The complete initial sync was tested both on M1 32GB RAM and 64 GB RAM. There were moments when no RAM was left on the 32 one.
There are packages that created problems on cargo test on mac os but worked using a docker image with linux. Those are:

  • stackslib and stacks-node on develop and next
  • blockstack-core stacks-node on master

Checklist

  • Fix mutation testing on packages to run on the supposed functions, not just on some
  • PR CI checker to have all functions tested
    • missed/unviable/timeout outputs result in an error and stop the PR from being merged
    • outputs the functions that have to be tested
  • On merge on main/develop/next branch override (by appending) current output with the merged one
  • Mutation Testing Coverage for the already existing functions
    • Mutation testing triggering on git diff that didn't affect the functions, but affected the tests. (if the function already exists and is not modified and the tests that use it are created/updated, the functions will not be included in the cargo mutants)
  • Check it works correctly and has the mutation output in the GH Action

- dockerfile and shell script for specific packages
- ci.yml for diff on packages on PR
@CLAassistant
Copy link

CLAassistant commented Nov 24, 2023

CLA assistant check
All committers have signed the CLA.

@ASuciuX
Copy link
Contributor Author

ASuciuX commented Nov 24, 2023

Notes: The CI.yml workflow should run on the PR as observed here in the local fork. It is waiting for action from a maintainer. @obycode @kantai @jbencin can you please submit it for action?

@ASuciuX
Copy link
Contributor Author

ASuciuX commented Nov 24, 2023

Fix mutants not being discovered in certain files

Specific for clarity package

The cargo-mutants crate believed that due to the name libclarity.rs, the vm module was located within libclarity/vm folder. A fix to this was renaming the file libclarity.rs to mod.rs, and changing the path from ./src/libclarity.rs to ./src/mod.rs in ./clarity/Cargo.toml:18. With these modifications, the package compiles and tests successfully (running on 4 threads cargo mutants --package clarity -f signatures.rs -j 4).

Copy link
Contributor

@jbencin jbencin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add some documentation on the following:

  • What is mutation testing and mutants?
  • How to understand the output
  • How to distinguish success from failure, or to know when it's found a problem

I think it should go in docs/testing.md, but I'm not sure where Jude will want it

mutants-testing-general.sh Outdated Show resolved Hide resolved
mutants/clarity/mutants.out/caught.txt Outdated Show resolved Hide resolved
clarity/Cargo.toml Outdated Show resolved Hide resolved
mutants-testing-general.sh Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1f5cc3f) 62.42% compared to head (25ee142) 82.46%.
Report is 15 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #4089       +/-   ##
============================================
+ Coverage    62.42%   82.46%   +20.04%     
============================================
  Files          401      401               
  Lines       286383   286385        +2     
============================================
+ Hits        178775   236169    +57394     
+ Misses      107608    50216    -57392     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@saralab saralab requested review from kantai and wileyj November 28, 2023 15:21
@ASuciuX
Copy link
Contributor Author

ASuciuX commented Dec 11, 2023

To be merged after stacks-network/actions#3 is merged. A new version of cargo mutants was released https://github.com/sourcefrog/cargo-mutants/releases/tag/v23.12.0 and it required a new sync from zero. After the sync is complete, will update the data on the action's repo and merge it.

@@ -15,7 +15,7 @@ resolver = "2"

[lib]
name = "clarity"
path = "./src/libclarity.rs"
path = "./src/lib.rs"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without changing this the cargo mutants command would not see the content of the files and call the mutations for them

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a bug in cargo-mutants. I'd open an issue on their GH. It seems related to cargo-mutants' specific file recognition patterns. Rust typically looks for src/lib.rs as the main entry point for a crate, so unconventional filenames might not be detected by cargo-mutants. That's my guess.

@ASuciuX
Copy link
Contributor Author

ASuciuX commented Dec 13, 2023

Can you also add some documentation on the following:

  • What is mutation testing and mutants?
  • How to understand the output
  • How to distinguish success from failure, or to know when it's found a problem

I think it should go in docs/testing.md, but I'm not sure where Jude will want it

I've added some details on this PR and on the PR in the actions repo. As it is completely automated through the CI workflows I think it should be in the actions repo, but I am open to suggestions. Also, not sure how in-depth to go in it as it mostly answers the questions above because of the implementation I've done in order to have straightforward responses based on what was the result of the mutations and also detail which are the functions not tested properly, in case there are.

@ASuciuX
Copy link
Contributor Author

ASuciuX commented Dec 15, 2023

Split this into 2 PRs as one could be merged and would be useful as soon as possible ( the filter one ) and waiting on the other one for the sync to be finished.

@obycode obycode marked this pull request as draft December 19, 2023 15:34
@ASuciuX ASuciuX changed the title Mutation Testing ( WIP - do not merge yet) Mutation Testing ( splitted into 2 PRs ) Jan 16, 2024
@ASuciuX ASuciuX self-assigned this Jan 16, 2024
@ASuciuX ASuciuX closed this Jan 16, 2024
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants