-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
CI speed-ups #706
Merged
Merged
CI speed-ups #706
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
Useful e.g. for comparing what got executed with different runs. Needs --first-is-1 for parallel rspec, which also fixes the first group prefix to be [TEST GROUP 1] instead of [TEST GROUP ]
Speeds up tests
Speeds up tests
Speeds up tests
Speeds up tests
Speeds up tests
Speeds up tests
…ase fails Helps with debugging in cases where a shutdown in a prior run somehow goes wrong, and web server or PHP-FPM fail to start again (since something is already/still listening on the port).
The purpose of this 'program' is to wait for output from a launched program, but with a timeout to prevent hanging forever. Its key feature is the ability to detect when it is part of a pipeline, and to then write something to that pipeline to signal that the expected output is there - this allows constructs like 'waitforit.sh 10 someprogram | { read; do_something_with_someprogram; }'. The previous solution for the internal pipeline signaling is, however, subject to certain race conditions, and then programs might not be terminated correctly, or the pipe signal loop may hang forever. It's simpler overall to just launch two 'timeout's, for the program and for the output grepping, and wait for either or both to terminate.
Speeds stuff up for composer_spec on heroku-22
For better parallelism
Speeds things up for ci_spec
Those are the slowest tests, so it helps a lot with overall time and with parallelization
It's where it belongs really (gets .gitignore-d there, too).
obsolete since fb28585
edmorley
approved these changes
Mar 8, 2024
Merged
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.
The Hatchet tests do a lot of
heroku run
s to check concurrency calculation, config handling, and other basic stuff on boot. They are performed for both Apache HTTPD and Nginx, with and without blackfire and newrelic installed, etc etc.This causes a lot of app creations, and a lot of
heroku run
calls, which overall, takes time.These changes collapse many of the expensive
runs
into a single one, and then split the output again to check the results in individual examples.Turns out the
waitforit.sh
test helper had a race condition (that did not affect us before since there weren't multiple "runs inside a run"), which is now fixed, too.In addition, some splitting of Composer and CI related tests brings the maximum test file time way down, and with
parallel_tests
grouping, the overall execution time drops significantly.The move to Hatchet 8 a few days ago already dropped the overall CI execution time from 6-7 minutes down to 5-6 minutes (since no app cleanups after the runs are necessary anymore).
Now we're consistenly in the 3-4 minute range, getting as low as 2:45 minutes.
The test diffs are obviously a bit terrible to read, since indentation changed, but not the test contents themselves. For easier review, I compared the list of tests that get executed, to demonstrate that nothing is accidentally dropped with this restructuring.
There are now ten more tests that get executed; condensed list for heroku-20:
The stack change cache test, now combined with another test, ran on heroku-22 only before, so it shows up for that stack as a deletion:
GUS-W-15212157