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

feat: mutation testing setup #1853

Merged
merged 11 commits into from
Jul 14, 2022
Merged

feat: mutation testing setup #1853

merged 11 commits into from
Jul 14, 2022

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Jun 22, 2022

First stab at mutation testing using the go-mutesting tool/binary.

A quick summary of mutation testing and the output the binary produces:

  • Mutation testing attempts to modify a program or binary, which are called mutants, and executes existing tests against those mutants.
  • If tests fail, the mutant is considered "killed".
  • Having mutants killed is a good thing, which means the software properly tests the various possible execution paths and should lead to strong confidence levels in the program's correctness.
  • The go-mutesting produces an output such as the following: The mutation score is 0.750000 (6 passed, 2 failed, 0 skipped, total is 8). This means 6 mutants were killed yielding a score or confidence level of 75%, which is not terrible. The higher the score the better.
  • False positives are probably going to be a common, i.e. a mutant that isn't killed but should be ignored. The most common example is "early exits" in a function/method. These are going to be pretty common I imagine and need to be added to a "blacklist" file that we will maintain as times goes on.

This PR sets up the scaffolding in CI for mutation testing and uses various parts of x/tokenfactory as the starting point. Subsequent PRs should be made which add more and more files/packages to be executed. Note, if the confidence score is low, that means our tests are not sufficient and should be improved prior to merging said PR.

@alexanderbez alexanderbez changed the title feat: feat: mutation testing setup Jun 22, 2022
mutation.blacklist Outdated Show resolved Hide resolved
Comment on lines 27 to 45
"workbench.colorCustomizations": {
"activityBar.activeBackground": "#403778",
"activityBar.activeBorder": "#000000",
"activityBar.background": "#403778",
"activityBar.foreground": "#e7e7e7",
"activityBar.inactiveForeground": "#e7e7e799",
"activityBarBadge.background": "#000000",
"activityBarBadge.foreground": "#e7e7e7",
"sash.hoverBorder": "#403778",
"statusBar.background": "#2d2755",
"statusBar.foreground": "#e7e7e7",
"statusBarItem.hoverBackground": "#403778",
"statusBarItem.remoteBackground": "#2d2755",
"statusBarItem.remoteForeground": "#e7e7e7",
"titleBar.activeBackground": "#2d2755",
"titleBar.activeForeground": "#e7e7e7",
"titleBar.inactiveBackground": "#2d275599",
"titleBar.inactiveForeground": "#e7e7e799"
},
Copy link
Member

Choose a reason for hiding this comment

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

Is there some sort of conflict happening with these colors? (Feels a bit odd to me to commit color preferences in the vscode config 😅 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to commit these for use with Peacock but oh well, reverted :(

# Only consider the following:
# * go files in types or keeper packages
# * ignore test and Protobuf files
MUTATION_SOURCES=$(find ./x -type f \( -path '*/keeper/*' -or -path '*/types/*' \) \( -name '*.go' -and -not -name '*_test.go' -and -not -name '*pb*' \))
Copy link
Member

Choose a reason for hiding this comment

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

This is some interestintg file specification syntax lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol this took me ages to figure out FYI

Makefile Outdated Show resolved Hide resolved
@ValarDragon
Copy link
Member

This is awesome! Ran the script for the token factory module, and this seems like qutie valid testing gaps found!

(That we currently aren't verifying codec changes, which even led to concerns on PRs #1972 #1994 )

Only errror type I;'m struggling to understand right now is:

-
-	_, err = GetTokenDenom(m.Sender, m.Subdenom)
+	_, _, _ = err, m.Sender, m.Subdenom
 	if err != nil {
 		return sdkerrors.Wrap(ErrInvalidDenom, err.Error())
 	}

wdyt the steps here should be? (Beyond understanding ^ and if its a false positive)

I feel like going ahead and merging this, and then maybe making CI run this on a schedule (perhaps once a week at first?) for a couple modules, and generate a list of failures into a github issue we tackle? (If details of that are hard, we can do so manually first couple of times)

@alexanderbez alexanderbez marked this pull request as ready for review July 13, 2022 20:30
@alexanderbez alexanderbez requested a review from a team July 13, 2022 20:30
@czarcas7ic
Copy link
Member

@ValarDragon simulation is failing in CI (and locally) every now and then when the simulator decides to use the same lock ID twice. I can look into it, but for now if we rerun this should clear

@ValarDragon ValarDragon merged commit 1486c84 into main Jul 14, 2022
@ValarDragon ValarDragon deleted the bez/mutant-testing-tf branch July 14, 2022 20:15
@ValarDragon
Copy link
Member

Really excited about this!

p0mvn pushed a commit that referenced this pull request Jul 18, 2022
Use make test-mutation

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants