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 tests only #38

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Mutation tests only #38

wants to merge 7 commits into from

Conversation

JekaMas
Copy link
Contributor

@JekaMas JekaMas commented Nov 4, 2022

Description

As far as consensus is extremely sensitive functionality, I believe we need a way to check our tests for usefulness and correctness. Code coverage, unfortunately, doesn't tell us about the quality of tests, either about usefulness. Even with high code coverage it's possible to write not very useful tests that always 'green', or tests that are 'green' if being executed in particular order.
For that reason the PR introduces changes:

  1. As far as we run tests with t.Parallel() it makes sense to use -race flag as well. At the moment tests do have data races that'd be great to fix in separate issues.
  2. To prevent incorrect global state the PR introduces -shuffle flag
  3. To check that tests are useful the PR adds mutation testing https://github.com/avito-tech/go-mutesting

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have added sufficient documentation in code

Comments

This PR is for discussion about new tooling. At the moment data race tests and mutation tests are red. If we'd like to merge the PR, we should comment out this CI steps and make tasks to fix tests with race and a bunch of tasks to add/check tests with respect of mutation testing results.

A few examples of mutation testing:

 func (i *IBFT) handlePrepare(view *proto.View, quorum uint64) bool {
 	isValidPrepare := func(message *proto.Message) bool {
 		// Verify that the proposal hash is valid
@@ -799,7 +799,7 @@
 
 	if len(prepareMessages) < int(quorum)-1 {
 		//	quorum not reached, keep polling
-		return false
+
 	}
 
func (i *IBFT) handlePrepare(view *proto.View, quorum uint64) bool {
 	isValidPrepare := func(message *proto.Message) bool {
 		// Verify that the proposal hash is valid
@@ -852,7 +852,7 @@
 		case <-sub.SubCh:
 			if !i.handleCommit(view, quorum) {
 				//	quorum not reached, retry
-				continue
+
 			}
 

 func (i *IBFT) handleCommit(view *proto.View, quorum uint64) bool {
 	isValidCommit := func(message *proto.Message) bool {
 		var (
@@ -880,7 +880,7 @@
 	commitMessage
[result.txt](https://github.com/0xPolygon/go-ibft/files/9723038/result.txt)
s := i.messages.GetValidMessages(view, proto.MessageType_COMMIT, isValidCommit)
 	if len(commitMessages) < int(quorum) {
 		//	quorum not reached, keep polling
-		return false
+
 	}
 


 func (i *IBFT) runStates(ctx context.Context) {
 	var timeout error
 
@@ -594,7 +594,7 @@
 
 	//	is proposer
 	if !i.backend.IsProposer(msg.From, height, round) {
-		return false
+
 	}

 func (i *IBFT) runStates(ctx context.Context) {
 	var timeout error
 
@@ -615,7 +615,7 @@
 
 	//	proposal must be for round 0
 	if msg.View.Round != 0 {
-		return false
+
 	}
 
 func (i *IBFT) handlePrePrepare(view *proto.View) *proto.Message {
 	isValidPrePrepare := func(message *proto.Message) bool {
 		if view.Round == 0 {
@@ -772,7 +772,7 @@
 		case <-sub.SubCh:
 			if !i.handlePrepare(view, quorum) {
 				//	quorum of valid prepare messages not received, retry
-				continue
+
 			}
 

More results are in the file result.txt

@JekaMas JekaMas added the feature New functionality label Nov 4, 2022
@JekaMas JekaMas self-assigned this Nov 4, 2022
@JekaMas JekaMas mentioned this pull request Nov 4, 2022
8 tasks
@codecov
Copy link

codecov bot commented Nov 4, 2022

Codecov Report

Merging #38 (da4fb4f) into main (12435ef) will not change coverage.
The diff coverage is n/a.

❗ Current head da4fb4f differs from pull request most recent head 428332e. Consider uploading reports for the commit 428332e to get more accurate results

@@           Coverage Diff           @@
##             main      #38   +/-   ##
=======================================
  Coverage   95.51%   95.51%           
=======================================
  Files           6        6           
  Lines        1292     1292           
=======================================
  Hits         1234     1234           
  Misses         42       42           
  Partials       16       16           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

.github/mut_config.yml Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@JekaMas JekaMas force-pushed the mutation-tests-only branch from a1e44e2 to 78ca14e Compare November 11, 2022 09:06
@JekaMas JekaMas requested review from vcastellm and removed request for zivkovicmilos January 9, 2023 08:50
* Minor fixes in ibft and tests (#45)

* Add rapid test with bad proposal coming from byzantine node (#44)

* Implement event generator for rapid testing (#46)

* EVM-220 TestClusterBlockSync/BLS fails in voting power branch (#48)

* Added per round event-based setup in rapid tests (#47)

* Remove redundant changeState (#49)

* Fix  Wrong Round Value in Validation of roundsAndPreparedBlockHashes (#51)

Fix round in roundsAndP reparedBlockHashes

* Audit improvements (#50)

* Audit improvements

* Add unit tests for EventManager to improve MSI

* Add unit tests for message helper to improve MSI

* Byzantine tests (#56)

* Byzantine tests

* Add unit tests for Messages to improve MSI

* Fix lint error

* fix lint errors only for the codes that changed in the PR

* fixed some stuck test

* Revert disabling function-length check in golangci

* Revert "Revert disabling function-length check in golangci"

This reverts commit 415633e.

Co-authored-by: Roman Behma <[email protected]>
Co-authored-by: Igor Crevar <[email protected]>
Co-authored-by: Vuk Gavrilovic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants