-
Notifications
You must be signed in to change notification settings - Fork 179
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
Increase size of CI runner to fix system tests #483
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krschacht
changed the title
Precompile assets before running system tests
Increase size of CI runner to fix system tests
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A couple days ago we started getting randomly failures like this:
I tried catching the exception and retrying, but the exception occurs in different parts of the code so that's not easy and probably not the right solution. I tried precompiling assets since some posts suggested this would fix, it didn't help. I spent awhile figuring out how to increase the Capybara timeout since various things online seemed to suggest that part of initializing was taking too long. I finally figured it out with this trail of clues:
driven_by
directive is defined: https://github.com/rails/rails/blob/9ba208c16835f4a174ae9fd385ebc18972d758a4/actionpack/lib/action_dispatch/system_test_case.rb#L158options
parameter is passed on through to the driver: https://github.com/rails/rails/blob/main/actionpack/lib/action_dispatch/system_testing/driver.rb#L56options[:timeout]
value is read by Capybara: https://github.com/teamcapybara/capybara/blob/0480f90168a40780d1398c75031a255c1819dce8/lib/capybara/selenium/driver.rb#L67read_timeout:
is set from the initializer: https://github.com/SeleniumHQ/selenium/blob/trunk/rb/lib/selenium/webdriver/remote/http/default.rb#L36But that didn't fix either. Finally, I increased the size of the github action runner just for system tests and that seems to have fixed it. I tried going back to the previous runner but with parallelization turned down to just 1 thread and that still failed.
I'm not sure why this would start happening all of the sudden. Maybe the overall resources that app is using have increased? Maybe github decreased the default (i.e. free) action runner size?
I take that back: the tests ran successfully twice in a row but then started timing out again. In the end, we're using the larger runners (4 cores) but I set parallelization to 2 and I increased the timeout just in case.