-
Notifications
You must be signed in to change notification settings - Fork 140
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
Various regression tests #1425
Various regression tests #1425
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1425 +/- ##
==========================================
+ Coverage 73.00% 73.14% +0.14%
==========================================
Files 288 282 -6
Lines 39786 39067 -719
==========================================
- Hits 29044 28575 -469
+ Misses 9266 9045 -221
+ Partials 1476 1447 -29
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit 242fcfa Results
|
runtime/various_test.go
Outdated
"github.com/onflow/cadence/runtime/sema" | ||
) | ||
|
||
func TestVariousAgainstRegression(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to move each test to the related component (parser/lexer, sema/checker, interpreter). This allows running the component's tests individually and detect regressions in one component earlier in the development. For example, if we were to break the parser, all parser tests could pass, and then only when we would run the runtime tests, we would realize there is a regression.
For example, the first test case is a parser test case, so it should go into runtime/parser2/parser_test.go
.
The next five test cases are checker tests, so they should go into runtime/tests/checker
.
For the test cases where no error is expected, the tests should be "duplicated" in both the checker tests and the interpreter tests, so we can ensure they are successfully checked, but then also successfully interpreted (other successful tests are "duplicated" in a similar way).
Sorry I was not clear about where to add the tests to begin with, I will document this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@turbolent I moved the tests around and restructured them to fit with the other tests in those files.
19a32a3
to
f67db5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you for adding these and putting them into the related "test suites" 👍
Only got a couple minor naming suggestions for keeping consistency, nothing major
Co-authored-by: Bastian Müller <[email protected]>
Co-authored-by: Bastian Müller <[email protected]>
Co-authored-by: Bastian Müller <[email protected]>
Closes https://github.com/dapperlabs/cadence-private-issues/issues/29
Description
Adds regression tests for Cadence. These bugs were found by Joe Soroka's fuzz testing.
master
branchFiles changed
in the Github PR explorer