-
Notifications
You must be signed in to change notification settings - Fork 674
ci: simplify test scripts and separate coverage from test runs #2566
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
base: main
Are you sure you want to change the base?
Changes from all commits
768f391
7c42d88
01c5512
8ad082c
f2ed2a0
ef719c3
279cda0
af3165f
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,4 @@ | ||
| --- | ||
| --- | ||
|
|
||
| ci: simplify test scripts and separate coverage from test runs |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,9 @@ on: | |||||||
| - main | ||||||||
| pull_request: | ||||||||
|
|
||||||||
| env: | ||||||||
| LATEST_SUPPORTED_NODE: "24.x" | ||||||||
|
Member
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. 🧠 praise: Super nice to surface this clear! |
||||||||
|
|
||||||||
| jobs: | ||||||||
| test: | ||||||||
| timeout-minutes: 6 | ||||||||
|
|
@@ -64,15 +67,14 @@ jobs: | |||||||
| run: npm run lint | ||||||||
| - name: Build docs | ||||||||
|
Member
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.
Suggested change
⌛ thought: We can save ~20 seconds for most runs with this? But perhaps it's adjacent to this PR! |
||||||||
| run: npm run docs | ||||||||
| - name: Run tests (Node 18/20) | ||||||||
| if: matrix.node-version != '22.x' && matrix.node-version != '24.x' | ||||||||
| # Node 18 lacks --test-reporter; Node 20 has coverage bugs. Use simpler script. | ||||||||
| run: npm run test:node18 --workspaces --if-present | ||||||||
| - name: Run tests (Node 22+) | ||||||||
| if: matrix.node-version == '22.x' || matrix.node-version == '24.x' | ||||||||
| - name: Run tests | ||||||||
| if: matrix.node-version != env.LATEST_SUPPORTED_NODE | ||||||||
| run: npm test | ||||||||
| - name: Run test coverage | ||||||||
| if: matrix.node-version == env.LATEST_SUPPORTED_NODE | ||||||||
| run: npm run test:coverage | ||||||||
| - name: Upload code coverage | ||||||||
| if: matrix.node-version == '24.x' && matrix.os == 'ubuntu-latest' | ||||||||
| if: matrix.node-version == env.LATEST_SUPPORTED_NODE && matrix.os == 'ubuntu-latest' | ||||||||
| uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6.0.0 | ||||||||
| with: | ||||||||
| fail_ci_if_error: true | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| "lint": "npx @biomejs/biome check packages", | ||
| "lint:fix": "npx @biomejs/biome check --write packages", | ||
| "test": "npm test --workspaces --if-present", | ||
| "test:coverage": "npm run test:coverage --workspaces --if-present", | ||
|
Member
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. 🏆 praise: These patterns are joined well here! |
||
| "version": "npm run changeset version && npm install && npm run docs" | ||
| }, | ||
| "devDependencies": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -37,9 +37,8 @@ | |||||
| "scripts": { | ||||||
| "build": "shx chmod +x src/*.js", | ||||||
| "prelint": "tsc --noemit --module es2022 --maxNodeModuleJsDepth 0 --project ./jsconfig.json", | ||||||
| "test": "npm run test:unit", | ||||||
| "test:node18": "bash -c 'node --test --test-reporter=spec src/*.test.js'", | ||||||
| "test:unit": "node --experimental-test-coverage --test-reporter=spec --test-reporter-destination=stdout --test-reporter=lcov --test-reporter-destination=lcov.info --test-reporter=junit --test-reporter-destination=test-results.xml --test src/*.test.js" | ||||||
| "test": "bash -c 'node --test-reporter=spec --test-reporter-destination=stdout --test-reporter=junit --test-reporter-destination=test-results.xml --test src/*.test.js'", | ||||||
|
Member
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.
Suggested change
🧪 thought(non-blocking): I like the pattern of a minimal |
||||||
| "test:coverage": "node --experimental-test-coverage --test-reporter=spec --test-reporter-destination=stdout --test-reporter=lcov --test-reporter-destination=lcov.info --test-reporter=junit --test-reporter-destination=test-results.xml --test src/*.test.js" | ||||||
| }, | ||||||
| "bin": { | ||||||
| "slack-cli-check-update": "src/check-update.js", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,7 +225,7 @@ expectAssignable<Parameters<typeof web.chat.appendStream>>([ | |
|
|
||
| ```bash | ||
| npm test --workspace=packages/web-api # all tests | ||
| npm run test:unit --workspace=packages/web-api # unit only | ||
| npm test --workspace=packages/web-api # unit only | ||
|
Comment on lines
227
to
+228
Member
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. 🔭 thought: One of these might be good to remove since it's unclear which test suite is used at a glance? |
||
| npm run test:types --workspace=packages/web-api # tsd only | ||
| ``` | ||
|
|
||
|
|
||
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.
🔭 note: I'm so curious for what happens to this file in upcoming releases!