-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
tests(ci): run unit tests on all active node versions #12513
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
name: unit | ||
|
||
on: | ||
push: | ||
branches: [master] | ||
pull_request: # run on all PRs, not just PRs to a particular branch | ||
|
||
jobs: | ||
# `unit` includes just unit and proto tests. | ||
unit: | ||
strategy: | ||
matrix: | ||
node: ['12', '14', '16'] | ||
runs-on: ubuntu-latest | ||
name: node ${{ matrix.node }} | ||
env: | ||
LATEST_NODE: '16' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICT it's not possible to refer to the |
||
|
||
steps: | ||
- name: git clone | ||
uses: actions/checkout@v2 | ||
with: | ||
# Depth of at least 2 for codecov coverage diffs. See https://github.com/GoogleChrome/lighthouse/pull/12079 | ||
fetch-depth: 2 | ||
|
||
- name: Use Node.js ${{ matrix.node }} | ||
uses: actions/setup-node@v1 | ||
with: | ||
node-version: ${{ matrix.node }} | ||
|
||
- name: Set up protoc | ||
uses: arduino/setup-protoc@64c0c85d18e984422218383b81c52f8b077404d3 | ||
with: | ||
version: '3.7.1' | ||
repo-token: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Set up Python | ||
uses: actions/setup-python@v1 | ||
with: | ||
python-version: 2.7 | ||
- name: Install Python dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install protobuf==3.7.1 | ||
|
||
- run: yarn install --frozen-lockfile --network-timeout 1000000 | ||
|
||
- run: yarn test-proto # Run before unit-core because the roundtrip json is needed for proto tests. | ||
|
||
- run: sudo apt-get install xvfb | ||
|
||
- name: yarn unit | ||
if: ${{ matrix.node != env.LATEST_NODE }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only running coverage on latest node means better coverage (since it'll have the latest v8 coverage updates) and saves ~3 minutes on the unit run for those not doing coverage |
||
run: xvfb-run --auto-servernum yarn unit:ci | ||
|
||
# Only gather coverage on latest node, where c8 is the most accurate. | ||
- name: yarn unit:coverage | ||
if: ${{ matrix.node == env.LATEST_NODE }} | ||
run: | | ||
xvfb-run --auto-servernum yarn unit:cicoverage | ||
yarn c8 report --reporter text-lcov > unit-coverage.lcov | ||
- name: Upload test coverage to Codecov | ||
if: ${{ matrix.node == env.LATEST_NODE }} | ||
uses: codecov/codecov-action@6004246f47ab62d32be025ce173b241cd84ac58e | ||
with: | ||
flags: unit | ||
file: ./unit-coverage.lcov | ||
|
||
# For windows, just test the potentially platform-specific code. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the existing |
||
unit-windows: | ||
runs-on: windows-latest | ||
name: Windows | ||
|
||
steps: | ||
- name: git clone | ||
uses: actions/checkout@v2 | ||
|
||
- name: Use Node.js 12.x | ||
uses: actions/setup-node@v1 | ||
with: | ||
node-version: 12.x | ||
|
||
- run: yarn install --frozen-lockfile --network-timeout 1000000 | ||
|
||
- name: yarn unit-cli | ||
run: yarn unit-cli | ||
- run: yarn diff:sample-json | ||
|
||
# Fail if any changes were written to any source files or generated untracked files (ex, from -GA). | ||
- run: git add -A && git diff --cached --exit-code |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ describe('locales', () => { | |
in: 'id', | ||
iw: 'he', | ||
mo: 'ro', | ||
tl: 'fil', | ||
connorjclark marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in node 16, the updated ICU version gives |
||
}; | ||
|
||
for (const locale of Object.keys(locales)) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we just run on
['12', '16']
? Not sure if there's a lot of downside to running on all three besides more jobs queued up and more chances for flakes