Skip to content

Commit 31b3949

Browse files
committed
ci: don't retry integration, unit test suites 3x in CI
This updates the unit test and integration test CI workflows to no longer retry the entire test suite when one or more test suites fails. This is in place to compensate for flaky test suites, but this approach makes individual test suites take up to 15 minutes when a test is legitimately failing and masks which tests are actually flaky. I've added inline retry configurations to known flaky tests and slapped a `--retry=3` on the integration test suite to cover the rest of the flaky integration tests (many of them are flaky due to port conflicts). As time goes on, hopefully we can fix and/or inline-annotate the remaining test flakes and get rid of the global retry config on integration tests. In the interim, this may cause an uptick in CI flakes, but I think after a week or two we'll be able to root out test flakes and make CI quicker and more reliable. Once I did this, the `get-global-config` unit tests started reliably failing on Windows. I looked into it and discovered that the first unit test run has always been failing on Windows. Adding test-specific retries did not fix it; I assume that the `get-global-config` test suite's setup is inadequate and indirectly relies on another test correctly setting up the filesystem. I fixed this by converting the `get-global-config` tests to use a mocked in-memory filesystem. When I couldn't get `fs` mocks to work reliably, I looked into `configstore` and discovered it's extremely simple and has a few dependencies we could get rid of, so I just rewrote it.
1 parent 11a8bfd commit 31b3949

17 files changed

+706
-246
lines changed

.github/workflows/integration-tests.yml

+2-12
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,7 @@ jobs:
7171
run: npm run test:init
7272

7373
- name: Tests
74-
uses: nick-fields/retry@v3
75-
with:
76-
timeout_minutes: 15
77-
max_attempts: 3
78-
retry_on: error
79-
command: npm run test:integration -- --coverage --shard=${{ matrix.shard }}
74+
run: npm run test:integration -- --coverage --shard=${{ matrix.shard }}
8075
env:
8176
# GitHub secrets are not available when running on PR from forks
8277
# We set a flag so we can skip tests that access Netlify API
@@ -164,12 +159,7 @@ jobs:
164159
run: npm run test:init
165160

166161
- name: Tests
167-
uses: nick-fields/retry@v3
168-
with:
169-
timeout_minutes: 15
170-
max_attempts: 3
171-
retry_on: error
172-
command: npm exec vitest -- run tests/integration/commands/dev/dev.test.ts --coverage
162+
run: npm exec vitest -- run tests/integration/commands/dev/dev.test.ts --coverage
173163
env:
174164
# GitHub secrets are not available when running on PR from forks
175165
# We set a flag so we can skip tests that access Netlify API

.github/workflows/unit-tests.yml

+1-6
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,4 @@ jobs:
4040
run: npm run build
4141

4242
- name: Run unit tests
43-
uses: nick-fields/retry@v3
44-
with:
45-
timeout_minutes: 15
46-
max_attempts: 3
47-
retry_on: error
48-
command: npm run test:unit -- --coverage
43+
run: npm run test:unit -- --coverage

__mocks__/fs.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// eslint-disable-next-line no-undef, @typescript-eslint/no-require-imports
2+
const { fs } = require('memfs')
3+
4+
// eslint-disable-next-line no-undef
5+
module.exports = fs

__mocks__/fs/promises.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// eslint-disable-next-line no-undef, @typescript-eslint/no-require-imports
2+
const { fs } = require('memfs')
3+
4+
// eslint-disable-next-line no-undef
5+
module.exports = fs.promises

__mocks__/write-file-atomic.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// eslint-disable-next-line no-undef, @typescript-eslint/no-require-imports
2+
const { fs } = require('memfs')
3+
4+
export const sync = fs.writeFileSync
5+
export default fs.writeFile

0 commit comments

Comments
 (0)