Always compute sha1 in lab and screenshot workflows in advance. A symbolic ref can change mid-workflow if the repo changes.
Co-authored-by: Aviv Keller <38299977+RedYetiDev@users.noreply.github.com>
When tests occasionally timeout (as is currently happening with
IndexedDB hangs on Mac), we should stop the test run after 30 minutes
and allow the "deflake" workflow to rerun them. Without this timeout,
the test run can takes hours.
A successful test run takes 15-20 minutes currently.
The condition for tagging the main branch was inverted after v4.7.11.
The main-branch tags for v4.6.16, v4.6.17, v4.7.12, v4.7.13, and v4.8.1
all had to be created and pushed by hand to correct this.
Since upgrading the deployment action from v0 to v2, and stumbling into
google-github-actions/deploy-appengine#361, new versions were not
properly promoted. However, our promotion setting in the workflow was
actually ignored the whole time due to a missing "echo".
Closes#6502
Upgrading release-please-action to v4 requires some changes:
- Move some settings from the action inputs to a manifest file (release-type, extra-files)
- Rename default-branch to target-branch
Additionally:
- There is an example in the new release-please-action docs that covers our exact use case for upload-release-action, but with the official GitHub CLI app. So we drop upload-release-action and use the GitHub CLI instead.
- There is an output in release-please-action that gives access to the patch version from semver. We should use this instead of running endsWith() over tag_name.
- Update mxschmitt/action-tmate@v3.6 => v3.17. This is a trivial update on an action that I missed in my last audit.
I missed some before. I only updated actions from GitHub "actions/..."
This updates all other actions except google-github-actions/release-please, which has some breaking changes we would need to absorb and test.
- Link CI status to specific jobs, not just the run
- Split selenium job status flag away from ignore_test_status (which is for getting screenshots)
- Remove redundant status setting step in update-screenshots
- Fix final status computation in update-screenshots to account for edge cases and old typo
This eliminates warnings about outdated node runtimes in GitHub Actions.
actions/download-artifact and actions/upload-artifact are the only ones
with any potential breaking changes, and adjustments have been made and
tested.
See
https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md
- Fix wrong index variable in loop ("i" vs "INDEX") that prevented
detection of commands at arbitrary places in a comment.
- Fix parsing of comments with newlines.
- Normalize commands to lowercase before checking them.
- Enhance debugging of command parsing.
- Allow the user to say "please" because it just feels nicer to me, even
when I'm talking to a robot.
lib/player.js was being updated separately because:
1. Originally, we didn't have support for updating arbitrary files with
release-please.
2. When we did get that support in release-please, it would trash the
"-uncompiled" tag we have in uncompiled mode.
By separating the uncompiled version string into two parts and using the
extra-files feature of release-please, we can get the updater to
preserve the "-uncompiled" tag and simplify the release workflow to only
update the PR once per change instead of twice.
In #5588 we switched off one part of using a fork for release PRs, but
not every part. That caused player.js not to be updated in release PRs,
which broke NPM releases.
This fixes the issue by removing other references to the shaka-bot fork.
Closes#5599
This enables workflows to be triggered automatically on release PRs,
instead of requiring maintainers to edit the PR description on every
release to trigger required PR workflows.
This uses a separate token from the other shaka-bot token, because it
requires additional permissions to make releases.
Now maintainers can send commands to Shaka Bot via PR comments.
Initially, Shaka Bot understands the following commands from anyone:
- `@shaka-bot help`: Show this help message
And the following commands from maintainers only:
- `@shaka-bot test`: Start lab tests on all devices
- `@shaka-bot test ce`: Start lab tests on CE devices only (no desktop
browsers)
This reverts commit 18ffa2cbbc.
So long as we make feature releases with a reasonable (< 100 PRs)
granularity, we won't need to keep doing this. Let's try to keep v4.4 an
anomaly.
GitHub started installing "Chrome for Testing" builds on Windows and
Mac. Unfortunately, these builds don't have support for H264 and AAC
codecs, which we need in our tests.
This fixes the issue by explicitly installing an official build of
Chrome on Windows, to overwrite the "Chrome for Testing" build.
This caches Babel's transpiler output for reuse, and should speed up all
test runs in theory, but the effect is most noticeable on local test
runs.
This uses a fork of karma-babel-preprocessor, which contains
https://github.com/babel/karma-babel-preprocessor/pull/77. If/when that
PR is merged, we can move back to the upstream module.
Local runs will start faster because only modified source files will be
re-processed through Babel when the tests start up.
In the Selenium workflow, Babel output and node_modules will both be
computed by the singular build-shaka job, stored, and then reused by all
the Selenium lab matrix jobs.
On my workstation (3.3 GHz cores, 32GB RAM, spinning platter disk), I
see tests start about ~60 seconds faster. In the lab (2.1-4.7 GHz cores,
64GB RAM, solid-state disk), I see tests start about ~10 seconds faster.
As I was building packages for shaka-lab services in the shaka-project/shaka-lab repo, it occurred to me that this KARMA_PORT variable was too specific to the shaka-player project. Instead, the new shaka-lab package for self-hosted runners will have a more generic ALLOCATED_PORT variable. This updates the CI workflow to use that new name.
When we first deployed certificates from letsencrypt, we did not know
about the "live" folder, which contains helpful symlinks to the latest
certificate. Instead, we were copying the entire "archive" folder, which
contains even old certs, then searching that folder for the latest
version. We should be using the "live" folder instead, letting certbot
maintain the symlinks. This means there is no need for
--lets-encrypt-folder in test.py.
- In karma.conf.js, give ourselves the ability to merge Selenium configs together without clobbering argument lists, for better deduplication in shaka-lab.yaml. This is based on https://github.com/shaka-project/karma-local-wd-launcher/pull/50
- Refactor shaka-lab.yaml to take advantage of this new ability. This removes a bunch of duplicate command line arguments for various versions of Chrome.
- Remove a TODO about a bug that Chrome decided they would never fix.
- Sort the list of devices and platforms, with comments to mark out the sections.
- Remove IE11, which we are well past supporting or testing.
This should be cherry-picked to all active branches.
This is a much simplified version of the appspot job from the release
workflow, but with control given to the human caller instead of computed
from release details. It can be used to correct deployment issues.
Due to limits in the number of versions you can have deployed at once on
App Engine, we can no longer keep every version of the Shaka Player Demo
deployed. So the version index needs to be aware of which versions are
deployed, and only link to versions that are still available.
This makes the index generator aware of App Engine deployments.
The initial commit was untested, but having the workflow in the main
branch allowed me to iterate on the workflow and test it in a branch.
The following changes were made:
- Standardize how workflow inputs are read (using inputs, which works
for all triggers, instead of github.event.inputs, which only works for
some triggers)
- Improve the description of test_filter in selenium-lab-tests.
- Add missing fields in the workflow_call trigger of
selenium-lab-tests.yaml. (Unfortunately, the two triggers need to have
the inputs defined separately, and with slightly different fields.)
- Log the chosen ref in selenium-lab-tests, to help debug inputs.
- Fix variable expansion in selenium-lab-tests so that test_filter can
contain spaces.
- Fix ignore_test_status, which did not work at all and broken the
nightly tests last night.
- Fix dependencies between jobs in update-screenshots.
- Do not set status for the subordinate selenium-lab-tests job if
started from update-screenshots.
- Always upload screenshots from selenium-lab-tests if started from
update-screenshots (normally only uploaded on failure).
- Set the PR status from upload-screenshots.
- Fix unpacking of screenshots from artifacts in upload-screenshots.
- Fix updating of changed screenshots.
- Fix handling of a lack of screenshot changes.
- Split up the jobs for easier debugging in upload-screenshots.
Still to do:
- Fix setting of final PR status
- Use a personal access token so that tests can be triggered from the PR
push
When the VTT size setting is used, the horizontal positioning was wrong
in both native and UI display.
The native display is wrong on Chrome and Edge because of a layout bug
in Chrome, where the shadow DOM for the cue box has conflicting
(redundant) styles. For example, these VTT settings:
`line: 85% position: 50% size: 63%`
result in these styles in the shadow DOM:
`top: 85%; left: 18.5%; transform: translate(-18.5%, -85%)`.
The `translate` style is what breaks the positioning. Unfortunately,
there is no way to fix that in JavaScript.
The UI display, however, was buggy for different reasons and is fixable.
The styles `left: 0; top: 0;` were applied by default, and then `top:
85%;` and `width: 63%;` were set based on the cue settings. The default
of `left: 0` was what broke the positioning. Removing this leaves `left`
set implicitly to `auto`, which is correct.
No other test cases were broken (or fixed) by this change.
This also adds a filter parameter to the lab workflow to run a subset of
tests for quicker results. This is useful for updating screenshots.
Closes b/259121343
This will upload screenshots when the Selenium lab tests fail. This can
be used to get screenshot changes either for examination or for updating
the test expectations.
There was a potential type error in an edge case in lib/player.js. It
would have been caught by the latest compiler, so this upgrades the
compiler and fixes another type error in one other place, as well.
This removes the overly-complex docker-compose system and rewrites the
Dockerfile. It also changes the docker commands to those that specify a
user ID, so that build outputs are written as the current user.
Closes#4387 (failed docker build not detected in CI)
Issue #4619 (failed to build w/ docker)
Because our release PRs are created with the default GitHub token, they
do not trigger test runs. This is a feature of GitHub that prevents
recursive loops of workflows.
This change makes it so that marking a PR "ready for review" will
trigger the tests to run again. This gives us an easy way to trigger
test runs on a release PR, by marking it a draft, then marking it
"ready" again.
Joey Parrish
Shaka Bot
Álvaro Velad Galván