medusa
medusa copied to clipboard
Use require package instead of assert while performing fs operations in unit tests
Instead of using the assert package, use the require package to stop testing if a test fails while performing filesystem operations (the assert package will only log the failure and continue execution).
Closes #86
I created a semgrep rule for this if we want to incorporate this tool into our CI.
rules:
- id: require-instead-of-assert
patterns:
- focus-metavariable: $E
- pattern-either:
- pattern:
$X, $E = $FSPACKAGE.$FUNC(...)
- pattern:
$E = $FSPACKAGE.$FUNC(...)
- pattern-either:
- pattern-inside: |
...
assert.$Y(..., $X)
- pattern-inside: |
...
assert.$Y(..., $E)
- metavariable-pattern:
metavariable: $FSPACKAGE
patterns:
- pattern-either:
- pattern: os
- pattern: utils # medusa specific
- pattern: ioutil
- pattern: filepath
message: Instead of using the `assert` package on the $E error variable, use the `require` package to stop testing
if a test fails while performing filesystem operations.
languages: [go]
severity: ERROR
I updated the branch here to target master.
Upon testing, we can't use require statements everywhere (in subtests, as it will FailNow() and then subsequent tests will fail for unrelated reasons: can't clean up the test's TempDir). We can probably use it in a few more places than this, but before merging this, I want to know why in the following code:
// Start the fuzzer
err := f.fuzzer.Start()
assert.NoError(t, err)
// Check for any failed tests and verify coverage was captured
assertFailedTestsExpected(f, true)
assertCorpusCallSequencesCollected(f, true)
The first assert.NoError() statement will fail an assertion, but the output of the failed assertion is not actually output if subsequent assertions (the two calls below) fail. That makes it a bit hard to capture the actual error on a failure.
I'll circle back to solving that, and then we can merge it. If you see an obvious solution (that doesn't require an if err != nil { continueProcessing(); }), then feel free to tackle it :)
The first
assert.NoError()statement will fail an assertion, but the output of the failed assertion is not actually output if subsequent assertions (the two calls below) fail. That makes it a bit hard to capture the actual error on a failure.I'll circle back to solving that, and then we can merge it. If you see an obvious solution (that doesn't require an
if err != nil { continueProcessing(); }), then feel free to tackle it :)
When the f.fuzzer.Events.FuzzerStopping event occurs, we can avoid printing output from the assert.NoError(t, err) and print only the two last ones.
Rough, but let me know what you think. fuzzing/fuzzer_test.go
func Test3(t *testing.T) {
runFuzzerTest(t, &fuzzerSolcFileTest{
filePath: "testdata/contracts/deployments/inner_inner_deployment.sol",
configUpdates: func(config *config.ProjectConfig) {
config.Fuzzing.DeploymentOrder = []string{"InnerDeploymentFactory"}
config.Fuzzing.Workers = 50
config.Fuzzing.TestLimit = 10000
},
method: func(f *fuzzerTestContext) {
// Start the fuzzer
err := f.fuzzer.Start()
err = fmt.Errorf("not very important bug happened")
expect := expectEventEmittedReturn(f, &f.fuzzer.Events.FuzzerStopping)
if !expect {
// Fuzzer did not stop properly, something bug happened
assert.NoError(t, err)
}
// Check for any failed tests and verify coverage was captured
if expect {
// Fuzzer stopped properly, and only these assertions matter
assertFailedTestsExpected(f, true)
assertCorpusCallSequencesCollected(f, true)
}
},
})
}
func expectEventEmittedReturn[T any](f *fuzzerTestContext, eventEmitter *events.EventEmitter[T]) bool {
eventType := eventEmitter.EventType().String()
eventEmitter.Subscribe(func(event T) error {
f.eventCounter[eventType] += 1
return nil
})
if f.eventCounter[eventType] == 0 {
return false
}
return true
}
I think this is a weird model for unit testing, to have to switch on each failure and not execute further code, so I'd prefer to find another solution here if we can. It was kind of the scenario I was trying to avoid with my comment regarding if err != nil { continueProcessing(); }.