Conversation
915370f to
ea95774
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3664 +/- ##
=======================================
Coverage 81.31% 81.31%
=======================================
Files 620 620
Lines 39075 39075
Branches 6351 6351
=======================================
Hits 31774 31774
Misses 6329 6329
Partials 972 972 ☔ View full report in Codecov by Sentry. |
ea95774 to
414661d
Compare
414661d to
f84959b
Compare
|
Given that servers have been upgraded, it probably makes sense to now review and merge this. |
| # Environments in which to run, such as those used in development and production, or which are candidates to | ||
| # move to. | ||
| os: ["ubuntu-22.04"] | ||
| os: ["ubuntu-24.04"] |
There was a problem hiding this comment.
🔴 chromium-browser command likely unavailable on ubuntu-24.04 runners, causing CI failure
The OS is changed from ubuntu-22.04 to ubuntu-24.04, but the "Pre-build report" steps at .github/workflows/build-and-test.yml:65-66 and .github/workflows/build-and-test.yml:180-181 still reference chromium-browser (via which chromium-browser and chromium-browser --version). On Ubuntu 24.04, the chromium-browser apt package became a snap transitional package, and GitHub Actions runners don't support snap well. The Chromium binary on ubuntu-24.04 is typically named chromium rather than chromium-browser. Because these steps run with set -xueo pipefail, a failed which chromium-browser command will immediately fail the entire job.
Affected lines in build-and-test.yml
Lines 65-66 (build-development job):
which chromium-browser
chromium-browser --versionLines 180-181 (build-production job):
which chromium-browser
chromium-browser --versionPrompt for agents
In .github/workflows/build-and-test.yml, the two "Pre-build report" steps (around lines 53-66 and 168-181) reference `chromium-browser`, which may not exist on ubuntu-24.04 GitHub Actions runners. Update lines 65-66 and 180-181 to use `chromium` instead of `chromium-browser`, or use a fallback like `which chromium-browser || which chromium` and `chromium-browser --version || chromium --version`. Also check the Ansible playbook at deploy/dev-server.playbook.yml:144 which sets `CHROME_BIN=chromium-browser` and may also need updating for ubuntu-24.04 compatibility in the e2e-tests.yml workflow.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The manifest shows that Chromium is at least in the Ubuntu 24.04 runner. Checking a runner's logs from this PR, we see
+ which chromium-browser
/usr/bin/chromium-browser
+ chromium-browser --version
Chromium 145.0.7632.0
So it looks like it is fine.
| # Environments in which to run, such as those used in development and production, or which are candidates to | ||
| # move to. | ||
| os: ["ubuntu-22.04"] | ||
| os: ["ubuntu-24.04"] |
There was a problem hiding this comment.
🚩 Incomplete OS migration across workflow files
Several other workflow files still reference ubuntu-22.04: compute-next-version.yml:32, release-live.yml:33,121, and release-qa.yml:29,76,142. This may be intentional (e.g., release workflows might need to stay on ubuntu-22.04 to match production, or may be updated in a separate PR), but it's worth confirming this is deliberate rather than an oversight.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Let's also update os in these remaining 3 files as well. Note that when we update os in release-live.yml and release-qa.yml, there might be initial problems deploying, because of default rsync arguments. So after we make that change I'd like to watch what happens when deploying to QA.
That change may as well be in this PR, but I won't hold up approval for that.
| strategy: | ||
| matrix: | ||
| os: ["ubuntu-22.04"] | ||
| os: ["ubuntu-24.04"] |
There was a problem hiding this comment.
🚩 Ansible playbook also references chromium-browser
The E2E tests workflow runs ansible-playbook dev-server.playbook.yml --limit localhost --diff at .github/workflows/e2e-tests.yml:446. The playbook at deploy/dev-server.playbook.yml:144 sets CHROME_BIN=chromium-browser. If chromium-browser doesn't exist on ubuntu-24.04, this would cause the karma test runner to fail to find a browser when running frontend tests in the E2E workflow. This is downstream of the same chromium-browser naming concern flagged as a bug, but affects a different workflow (e2e-tests.yml) via the Ansible playbook rather than directly in the workflow YAML.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I don't think it's a problem. And the E2E build passed.
marksvc
left a comment
There was a problem hiding this comment.
@marksvc reviewed 3 files and all commit messages, made 3 comments, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Nateowami).
| # Environments in which to run, such as those used in development and production, or which are candidates to | ||
| # move to. | ||
| os: ["ubuntu-22.04"] | ||
| os: ["ubuntu-24.04"] |
There was a problem hiding this comment.
Let's also update os in these remaining 3 files as well. Note that when we update os in release-live.yml and release-qa.yml, there might be initial problems deploying, because of default rsync arguments. So after we make that change I'd like to watch what happens when deploying to QA.
That change may as well be in this PR, but I won't hold up approval for that.
| # Environments in which to run, such as those used in development and production, or which are candidates to | ||
| # move to. | ||
| os: ["ubuntu-22.04"] | ||
| os: ["ubuntu-24.04"] |
There was a problem hiding this comment.
The manifest shows that Chromium is at least in the Ubuntu 24.04 runner. Checking a runner's logs from this PR, we see
+ which chromium-browser
/usr/bin/chromium-browser
+ chromium-browser --version
Chromium 145.0.7632.0
So it looks like it is fine.
| strategy: | ||
| matrix: | ||
| os: ["ubuntu-22.04"] | ||
| os: ["ubuntu-24.04"] |
There was a problem hiding this comment.
I don't think it's a problem. And the E2E build passed.
This PR exists to test what happens when we start running builds on Ubuntu 24.04. It exists as a test, rather than something we actually intend to merge.
This change is