-
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
CircleCI: parameterizing no_output_timeout and -short #2618
CircleCI: parameterizing no_output_timeout and -short #2618
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2618 +/- ##
==========================================
- Coverage 46.98% 46.97% -0.01%
==========================================
Files 348 348
Lines 55717 55717
==========================================
- Hits 26179 26174 -5
- Misses 26594 26601 +7
+ Partials 2944 2942 -2
Continue to review full report at Codecov.
|
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.
LGTM
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.
default: "/home/circleci" | ||
result_subdir: | ||
default: 30m | ||
short_test_flag: |
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 wonder if this could be made more generic to support any test flag?
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'm not sure I want to do that anymore:
- We're calling a bunch of other parameters inside the test itself right now, so it could be confusing to add an any_test flag because you can't add whatever parameter you want. Only parameters that haven't been specified yet
-short
is the main flag we're using and it helps to repeat the same variable name used in generic_integration for consistency- Will mentioned that he wants us to stop calling gotestsum directly in the file eventually, so I expect further modifications on this command. (1426 in our CircleCI epic)
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.
LGTM
Parameterize no_output_timeout and -short in the general commands on the circle config file to reduce duplicate code.
Summary
Parameterize no_output_timeout and -short in the general commands on the circle config file to reduce duplicate code.
I did also increase the size of integration tests to large as I saw that we had errors in mac_amd64_integration, arm64_integration and arm64_integration recently but would run after manually rerunning from failure.
Test Plan
Tested it on a Pull request that would run the nightly and then tested the regular short tests once.