-
Notifications
You must be signed in to change notification settings - Fork 493
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
Fix goalNodeTest.exp #2781
Fix goalNodeTest.exp #2781
Conversation
d8cec4c
to
fe8ac97
Compare
"pingpongTest.exp": true, // broken | ||
"listExpiredParticipationKeyTest.exp": true, // flaky |
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 would be better to modify the expect test itself to return right away - so that the "disable" is on the test itself and we don't need to maintain "good test/bad test" lists.
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.
The problem with this expected does not prints out output when it is successful so there is no way to know it did not run. With the current approach tho skipped files do not appear in go test -v
log. Plus I'll add logging here to emphasize it was skipped.
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.
What if the expect would output a "Skip" output ? we're already reading the output in the go-code, so we can just call the t.Skip in that case.
11b98b3
to
e5cbda4
Compare
* Disable listExpiredParticipationKeyTest as flaky one * Increase timeout in goalClerkGroupTest
e5cbda4
to
eb78908
Compare
@algorandskiy - it's great that you're enabling these tests.. unfortunately these still randomly fails ( even with your fixes ). |
Summary
Status code differs on CircleCI. Relaxed the status code value check.
Change the skipped expect tests to be defined as "t.Skip" so we can track these as such on the log file.
Test Plan
This is a test fix.