Skip to content
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 17 commits into from
Mar 8, 2024
Merged

CI speed-ups #706

merged 17 commits into from
Mar 8, 2024

Conversation

dzuelke
Copy link
Contributor

@dzuelke dzuelke commented Mar 8, 2024

The Hatchet tests do a lot of heroku runs 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:

diff --git a/h20 b/h20
index bde3d1ce..8e6eb777 100644
--- a/h20
+++ b/h20
@@ -1,0 +2 @@ A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/i
+A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/integration-heroku explicitly during boot logs info messages about agent startup
@@ -4,0 +6 @@ A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/i
+A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/integration-heroku with default BLACKFIRE_LOG_LEVEL during boot does not log info messages about agent startup
@@ -7,0 +10 @@ A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/i
+A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/integration-heroku implicitly during boot logs info messages about agent startup
@@ -10,0 +14 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package e
+A PHP application using ext-blackfire and, as its agent, our blackfire package explicitly during boot logs info messages about agent startup
@@ -13,0 +18 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package w
+A PHP application using ext-blackfire and, as its agent, our blackfire package without BLACKFIRE_SERVER_TOKEN during boot logs info messages about agent startup
@@ -16,0 +22 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package w
+A PHP application using ext-blackfire and, as its agent, our blackfire package with default BLACKFIRE_LOG_LEVEL during boot does not log info messages about agent startup
@@ -19,0 +26 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package i
+A PHP application using ext-blackfire and, as its agent, our blackfire package implicitly during boot logs info messages about agent startup
@@ -32 +39 @@ A PHP application on Heroku CI installs dev dependencies and caches them
-A PHP application on Heroku CI has zend.assertions enabled
+A PHP application on Heroku CI has zend.assertions enabled and auto-runs a composer.json 'test' script entry
@@ -34 +40,0 @@ A PHP application on Heroku CI fails to auto-run tests if nothing suitable is fo
-A PHP application on Heroku CI specifying a composer.json 'test' script entry executes 'composer test'
@@ -49,0 +56 @@ A PHP application using New Relic explicitly does not start New Relic daemon dur
+A PHP application using New Relic explicitly during boot logs info messages about daemon startup
@@ -53,0 +61 @@ A PHP application using New Relic without NEW_RELIC_LICENSE_KEY does not start N
+A PHP application using New Relic without NEW_RELIC_LICENSE_KEY during boot logs info messages about daemon startup
@@ -57,0 +66 @@ A PHP application using New Relic with default NEW_RELIC_LOG_LEVEL does not star
+A PHP application using New Relic with default NEW_RELIC_LOG_LEVEL during boot does not log info messages about daemon startup
@@ -61,0 +71 @@ A PHP application using New Relic implicitly does not start New Relic daemon dur
+A PHP application using New Relic implicitly during boot logs info messages about daemon startup
@@ -490 +500 @@ A PHP 8.3 application with long-running requests that uses PHP 8.3 and the nginx
-A PHP application like the Heroku Getting Started guide example for PHP deploys and works
+A PHP application like the Heroku Getting Started guide example for PHP deploys, works, and re-uses cached dependencies on stack change

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:

diff --git a/h22 b/h22
index 68194d67..7a48007c 100644
--- a/h22
+++ b/h22
@@ -1,0 +2 @@ A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/i
+A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/integration-heroku explicitly during boot logs info messages about agent startup
@@ -4,0 +6 @@ A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/i
+A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/integration-heroku with default BLACKFIRE_LOG_LEVEL during boot does not log info messages about agent startup
@@ -7,0 +10 @@ A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/i
+A PHP application using ext-blackfire and, as its agent, buildpack blackfireio/integration-heroku implicitly during boot logs info messages about agent startup
@@ -10,0 +14 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package e
+A PHP application using ext-blackfire and, as its agent, our blackfire package explicitly during boot logs info messages about agent startup
@@ -13,0 +18 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package w
+A PHP application using ext-blackfire and, as its agent, our blackfire package without BLACKFIRE_SERVER_TOKEN during boot logs info messages about agent startup
@@ -16,0 +22 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package w
+A PHP application using ext-blackfire and, as its agent, our blackfire package with default BLACKFIRE_LOG_LEVEL during boot does not log info messages about agent startup
@@ -19,0 +26 @@ A PHP application using ext-blackfire and, as its agent, our blackfire package i
+A PHP application using ext-blackfire and, as its agent, our blackfire package implicitly during boot logs info messages about agent startup
@@ -32 +39 @@ A PHP application on Heroku CI installs dev dependencies and caches them
-A PHP application on Heroku CI has zend.assertions enabled
+A PHP application on Heroku CI has zend.assertions enabled and auto-runs a composer.json 'test' script entry
@@ -34,2 +40,0 @@ A PHP application on Heroku CI fails to auto-run tests if nothing suitable is fo
-A PHP application on Heroku CI specifying a composer.json 'test' script entry executes 'composer test'
-A PHP application that has its stack changed re-uses cached dependencies from the prior stack build
@@ -47,0 +53 @@ A PHP application using New Relic explicitly does not start New Relic daemon dur
+A PHP application using New Relic explicitly during boot logs info messages about daemon startup
@@ -51,0 +58 @@ A PHP application using New Relic without NEW_RELIC_LICENSE_KEY does not start N
+A PHP application using New Relic without NEW_RELIC_LICENSE_KEY during boot logs info messages about daemon startup
@@ -55,0 +63 @@ A PHP application using New Relic with default NEW_RELIC_LOG_LEVEL does not star
+A PHP application using New Relic with default NEW_RELIC_LOG_LEVEL during boot does not log info messages about daemon startup
@@ -59,0 +68 @@ A PHP application using New Relic implicitly does not start New Relic daemon dur
+A PHP application using New Relic implicitly during boot logs info messages about daemon startup
@@ -280 +289 @@ A PHP 8.3 application with long-running requests that uses PHP 8.3 and the nginx
-A PHP application like the Heroku Getting Started guide example for PHP deploys and works
+A PHP application like the Heroku Getting Started guide example for PHP deploys, works, and re-uses cached dependencies on stack change

GUS-W-15212157

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 ]
…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
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).
@dzuelke dzuelke requested a review from a team as a code owner March 8, 2024 13:13
@dzuelke dzuelke merged commit e74f187 into main Mar 8, 2024
5 checks passed
@dzuelke dzuelke deleted the ci-herokurun-speedups branch March 8, 2024 16:14
@heroku-linguist heroku-linguist bot mentioned this pull request Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants