Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/nightly-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ jobs:
FIREFOX_NIGHTLY_BIN: ${{ steps.install-deps.outputs.firefox-path }}
run: npm run test -- --browsers FirefoxNightly
- name: Run Chrome Beta Browser Tests
if: ${{ always() }}
env:
CHROME_BIN: ${{ steps.install-deps.outputs.chrome-path }}
CHROMEDRIVER_BIN: ${{ steps.install-deps.outputs.chromedriver-path }}
Expand Down
222 changes: 193 additions & 29 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,50 +7,214 @@ on:
- master
- develop

concurrency:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we run on PRs and on pushes to key branches, the concurrency setup will run each push event one at a time in series. So everything will fully finish there. Meanwhile for PRs, if something is rapidly pushed to a PR branch, it will cancel the previous commits run since it is no longer useful to the PR and continue with running the latest commit on those branches.

This helps save resources for the project.

group: axe-core Tests ${{ github.ref_name }}
cancel-in-progress: ${{ github.event_name != 'push' }}

permissions:
contents: read

env:
CHROME_DEVEL_SANDBOX: /opt/google/chrome/chrome-sandbox

jobs:
build:
runs-on: ubuntu-latest
timeout-minutes: 5
lint:
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- uses: actions/checkout@v5
- uses: actions/setup-node@v6
- &checkout
name: Checkout repository
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
- &setup-node
name: Set up Node.js
uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 # v6.0.0
with:
node-version-file: .nvmrc
cache: 'npm'
- run: npm ci
- run: npm run prepare
- run: npm run build
- uses: actions/upload-artifact@v5
- &install-deps-directly
name: Install Dependencies
run: npm ci
- name: Run ESLint
run: npm run eslint

fmt_check:
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- *checkout
- *setup-node
- *install-deps-directly
- run: npm run fmt:check

build:
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- *checkout
- *setup-node
- *install-deps-directly
- &build
name: Build
run: |
npm run prepare
npm run build
- uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
with:
name: axe-core
path: axe.js
retention-days: 1

fmt_check:
runs-on: ubuntu-latest
timeout-minutes: 5
test_chrome:
runs-on: ubuntu-24.04
timeout-minutes: 10
env:
DISPLAY: :99
steps:
- uses: actions/checkout@v5
- uses: actions/setup-node@v6
- *checkout
- &install-deps-with-xvfb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understand everything correctly, calling &install-deps-with-xvfb will reinstall the node modules for every step and not cache it, whereas in circle we found a way to cache the build and the node modules so it only happened once for the entire run. This isn't so much a problem for now as as you said for the build we can worry about optimizing after. Looking at the runs, each of the tests take ~2mins longer than their circleCI counterpart, but since it all runs in parallel it doesn't add too much time in the long run.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That feels like a step back. I don't care if we do that in this PR or a separate one but I don't think we should call this done until we've sorted this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, cacheing of node modules is handled by actions/setup-node. It caches the local cache of node_modules and not the modules in the repo after installation. Reason being, just restoring the cache in the repo may miss running install scripts that make things work. (This is probably why some of the tests broke after the move.)

The build time is like 30 seconds of effort when we do need to re-run it. Not enough time to run up a bill too much.

Ideally, we'd ultimately end up building docker images we run against that have all the OS deps pre-installed (aside from nightly stuff which of course we need to install at runtime.) This would reduce the bulk of that extra ~2m time. Since most of it is spent downloading and installing the same OS packages on every runner.

No one that I'm aware of in the org has done Docker Images for CI yet in GHA. This "install deps" shared action is how every other project has done this that I've seen. So it's a trade-off the org has accepted for the initial conversion of these projects to GHA. If we want to prioritize setting up the infrastructure for making docker images and refreshing them regularly for CI to run against, I'm all for it. But that's a bigger task that will need to be planned out with PO buy-in so it has accounted time to work on it and get it done.

name: Install Deps
uses: ./.github/actions/install-deps
id: install-deps
with:
node-version-file: .nvmrc
cache: 'npm'
- run: npm ci
- run: npm run fmt:check
start-xvfb: ${{ env.DISPLAY }}
- *build
- name: Run Tests Against Chrome
env:
CHROME_BIN: ${{ steps.install-deps.outputs.chrome-path }}
CHROMEDRIVER_BIN: ${{ steps.install-deps.outputs.chromedriver-path }}
run: npm run test -- --browsers Chrome
- name: Run Chrome Integration Tests
env:
CHROME_BIN: ${{ steps.install-deps.outputs.chrome-path }}
CHROMEDRIVER_BIN: ${{ steps.install-deps.outputs.chromedriver-path }}
run: npm run test:integration:chrome

test_firefox:
runs-on: ubuntu-24.04
timeout-minutes: 10
env:
DISPLAY: :99
steps:
- *checkout
- *install-deps-with-xvfb
- *build
- name: Run Tests Against Firefox
env:
FIREFOX_BIN: ${{ steps.install-deps.outputs.firefox-path }}
run: npm run test -- --browsers Firefox
- name: Run Firefox Integration Tests
env:
FIREFOX_BIN: ${{ steps.install-deps.outputs.firefox-path }}
run: npm run test:integration:firefox

# Run examples under `doc/examples`
test_examples:
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- *checkout
- &install-deps
name: Install Deps
id: install-deps
uses: ./.github/actions/install-deps
- *build
- name: Run Tests Against Examples
run: npm run test:examples

test_act:
runs-on: ubuntu-24.04
timeout-minutes: 10
needs: build
steps:
- *checkout
- *install-deps
- &restore-axe-build
name: Restore axe build
uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0
with:
name: axe-core
- name: Run ACT Tests
env:
CHROME_BIN: ${{ steps.install-deps.outputs.chrome-path }}
CHROMEDRIVER_BIN: ${{ steps.install-deps.outputs.chromedriver-path }}
run: npm run test:act

test_aria_practices:
runs-on: ubuntu-24.04
timeout-minutes: 10
needs: build
steps:
- *checkout
- *install-deps
- *restore-axe-build
- name: Run ARIA Practices Tests
env:
CHROME_BIN: ${{ steps.install-deps.outputs.chrome-path }}
CHROMEDRIVER_BIN: ${{ steps.install-deps.outputs.chromedriver-path }}
run: npm run test:apg

test_locales:
runs-on: ubuntu-24.04
timeout-minutes: 10
needs: build
steps:
- *checkout
- *install-deps
- *restore-axe-build
- name: Run Locale Tests
run: npm run test:locales

test_virtual_rules:
runs-on: ubuntu-24.04
timeout-minutes: 10
needs: build
steps:
- *checkout
- *install-deps
- *restore-axe-build
- name: Run Virtual Rules Tests
run: npm run test:virtual-rules

test_jsdom:
runs-on: ubuntu-24.04
timeout-minutes: 10
needs: build
steps:
- *checkout
- *install-deps
- *restore-axe-build
- name: Run jsdom Tests
run: npm run test:jsdom

build_api_docs:
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- *checkout
- *install-deps
- name: Run API Docs Build
run: npm run api-docs

test_node:
# The package can't be built on Node 6 anymore, but should still run there.
# So we need to pull in a previous build artifact.
needs: build
strategy:
matrix:
node: [6, 18, 20, 22]
runs-on: ubuntu-latest
timeout-minutes: 5
needs: build
node:
- 6
- 18
- 20
- 22
- 24
runs-on: ubuntu-24.04
timeout-minutes: 10
steps:
- uses: actions/checkout@v5
- uses: actions/setup-node@v6
- *checkout
- name: Set up Node.js
uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 # v6.0.0
with:
node-version: ${{ matrix.node}}
- uses: actions/download-artifact@v6
with:
name: axe-core
- run: npm run test:node
node-version: ${{ matrix.node }}
- *restore-axe-build
- name: Run Node.js Tests
run: npm run test:node
2 changes: 1 addition & 1 deletion test/integration/full/test-webdriver.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function buildWebDriver(browser) {
const options = new firefox.Options().addArguments('--headless');

if (process.env.FIREFOX_BIN) {
options.setBinaryPath(process.env.FIREFOX_BIN);
options.setBinary(process.env.FIREFOX_BIN);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got confused somewhere and had the wrong method in here. It just wasn't hit in the nightly testing where I was setting this env change up since it used a different variable.

Why does Webdriver not have uniform method naming for doing the same action between browsers? No clue. But we also can't use // @ts-check in this file since so many other things are broken. That will need a bigger effort to go fix on its own to help avoid these issues.

}

webdriver = firefox.Driver.createSession(options);
Expand Down