build: use node test runner for workspace packages by zimeg · Pull Request #2529 · slackapi/node-slack-sdk

zimeg · 2026-03-03T07:11:53Z

Summary

This PR uses the node test runner for unit tests of workspace packages - #2359 🎁

$ npm test
$ npm test --workspace=@slack/web-api

Also removed are packages used for testing before for fewer dependencies overall:

Before: 628 packages
After: 507 packages

These include kind development dependencies: 👾 ✨

Notes

The testing matrix is reduced to 6 runners - one for each OS and node version - since unit tests for all packages take around one minute altogether. We hope to reduce computation time from 4820 seconds to much less!

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

changeset-bot · 2026-03-03T07:12:02Z

🦋 Changeset detected

Latest commit: 55cf42e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@slack/oauth	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2026-03-03T07:12:43Z

Codecov Report

❌ Patch coverage is 97.88918% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.66%. Comparing base (57f4f22) to head (55cf42e).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2529      +/-   ##
==========================================
- Coverage   93.17%   90.66%   -2.51%     
==========================================
  Files          40       82      +42     
  Lines       11384    14980    +3596     
  Branches      726      508     -218     
==========================================
+ Hits        10607    13582    +2975     
- Misses        765     1373     +608     
- Partials       12       25      +13

Flag	Coverage Δ
cli-hooks	`90.66% <97.88%> (-4.77%)`	⬇️
cli-test	`90.66% <97.88%> (-4.13%)`	⬇️
logger	`90.66% <97.88%> (?)`
oauth	`90.66% <97.88%> (+13.26%)`	⬆️
socket-mode	`90.66% <97.88%> (+28.78%)`	⬆️
web-api	`90.66% <97.88%> (-7.47%)`	⬇️
webhook	`90.66% <97.88%> (-6.00%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Replace hardcoded `/` path separators with path.join() and path.sep to fix test failures on Windows where path separators differ. Co-Authored-By: Claude <[email protected]>

srtaalej

thanks for maintaining our code health ⭐! looks like codecov is being finicky but would be great to land this once thats resolved 💫 🚀

srtaalej · 2026-03-03T16:09:44Z

zimeg

@srtaalej Thanks so much for reviewing this! I made a few more changes and left some comments of thoughts.

...codecov is being finicky

I'm noticing this too but think the most recent "error" is from test coverage decreasing. I understand the node test runner to be more honest about this so this is expected to me with these changes 🫡

Let's continue to improve code health so future changes can be made fast! I think this PR points at an area of @slack/socket-mode that we might revisit for example 🔍

zimeg · 2026-03-03T17:33:10Z

-    const tempDir = path.join(process.cwd(), 'tmp');
+    const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-update-'));


👁️‍🗨️ note: Here we're avoiding a stalemate when multiple tests run at the same time. Now we instead use separate tmp directories!

zimeg · 2026-03-03T17:35:04Z

+      sinon.stub(childProcess, 'exec').callsFake((command, cb) => {
+        cb(null, { stdout: mockNPM(command), stderr: '' });
      });


📺 note: We stub the exec process now instead of the waiting of it without needing to change calling code! This ought be a more true stub I think!

zimeg · 2026-03-03T17:44:43Z

    "rewiremock": "^3",
    "shx": "^0.4.0",
    "sinon": "^21",
-    "source-map-support": "^0.5.21",


🪓 note: It was unclear to me how source-map-support was being used at first, but I find this to exist alongside the c8 configurations that are also removed.

zimeg · 2026-03-03T17:46:52Z

    "sinon": "^21",
-    "source-map-support": "^0.5.21",
-    "ts-node": "^10",
+    "tsx": "^4.20.6",


🎁 note: We prefer tsx with these latest changes instead of ts-node for imports into the test:

📚 https://nodejs.org/en/learn/typescript/run

zimeg · 2026-03-03T18:12:26Z

+// Use require() so rewiremock can intercept @slack/web-api dependency
+var { InstallProvider } = require('./index') as typeof import('./index');


🧪 note: I'm not thrilled with unordered imports but this seems to be clear and meaningful to keep in tests, without updating the linting rules.

🗣️ ramble: We should perhaps consider changing the mocks we have to be at the Web API call and not method but that's outside the scope of these changes for now.

zimeg · 2026-03-03T18:29:31Z

-      writeToFile(`${installationDir}/app-latest`, installationData);
-      writeToFile(`${installationDir}/user-${user.id}-latest`, installationData);
+      writeToFile(path.join(installationDir, 'app-latest'), installationData);
+      writeToFile(path.join(installationDir, `user-${user.id}-latest`), installationData);


🗣️ note: This change is to production code to improve our tests on Windows! I added a changeset for this here in c3d1f1a.

zimeg · 2026-03-03T19:09:41Z

+      // Swallow unhandled rejections from internal reconnection cleanup
+      process.emit = (event, ...args) => {
+        if (event === 'unhandledRejection') return true;
+        return originalEmit(event, ...args);
+      };


👁️‍🗨️ note: Perhaps related to #1787 but let's continue investigation later. We added this to avoid flaky tests for now.

zimeg · 2026-03-03T19:10:25Z

      do {
-        await sleep(2);
+        await sleep(5);
        retries = closed;
        elapseTime = Date.now() - startTime;
-      } while (retries < 2 && elapseTime < 50);
-      // after less then 50 milliseconds, with a timeout of 20ms, we would expect 2 retries.
+      } while (retries < 2 && elapseTime < 2000);
+      // with a clientPingTimeout of 20ms, we would expect 2 retries well within 2 seconds.


📝 note: Here we're padding our possible timing for edges that might exist on Windows startup.

zimeg added 6 commits March 2, 2026 21:55

build: use node test runner for workspace packages

9b2de8f

chore: merge w main

c02a6ff

chore: agents

bbaafab

docs(maintainers): npm test

9b0924c

test(cli-hooks): use actual tmp to avoid race condition

f8f538b

build: remove source map support from development dependencies

6aef281

zimeg self-assigned this Mar 3, 2026

zimeg requested a review from a team as a code owner March 3, 2026 07:11

zimeg and others added 10 commits March 2, 2026 23:30

chore: lint

72a5125

style: lint

80d53c7

ci: run tests on earlier versions

acb2c2b

style: glob

5a15a5a

test(oauth): use path.join for cross-platform compatibility in tests

0db8c83

Replace hardcoded `/` path separators with path.join() and path.sep to fix test failures on Windows where path separators differ. Co-Authored-By: Claude <[email protected]>

ci: bash

6e161be

test(socket-mode): avoid rushing reconnect attempts

e1ea765

ci: coverage node 22

da55234

fix(oauth): use path.join for standard filepath operations

a06bca8

build: glob

f870dad

zimeg added 5 commits March 3, 2026 01:18

build: npx

51c25b8

build: glob dev deps package

33e1a01

build: globstar

a9f4fce

test: bash

2c2d297

ci: test results

858336a

srtaalej approved these changes Mar 3, 2026

View reviewed changes

Comment thread .claude/settings.json

Copy link
Copy Markdown

Contributor

srtaalej Mar 3, 2026

Choose a reason for hiding this comment

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

🤩

zimeg linked an issue Mar 3, 2026 that may be closed by this pull request

Fix flaky Windows 18.x and 22.x unit tests #2159

Closed

7 tasks

zimeg added 2 commits March 3, 2026 10:05

style: improve casting of expected values before assertion

3a5535e

docs: changeset

c3d1f1a

zimeg added the bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented label Mar 3, 2026

zimeg added 2 commits March 3, 2026 11:23

test: improve type assertions of expected values

25f2815

chore: lint

55cf42e

zimeg commented Mar 3, 2026

View reviewed changes

zimeg merged commit dbd38e2 into main Mar 3, 2026
14 of 16 checks passed

zimeg deleted the test branch March 3, 2026 19:39

zimeg mentioned this pull request Mar 3, 2026

Fix flaky Windows 18.x and 22.x unit tests #2159

Closed

7 tasks

		const tempDir = path.join(process.cwd(), 'tmp');
		const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-update-'));

		// Use require() so rewiremock can intercept @slack/web-api dependency
		var { InstallProvider } = require('./index') as typeof import('./index');

Conversation

zimeg commented Mar 3, 2026

Summary

Notes

Requirements

Uh oh!

changeset-bot Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

codecov Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

srtaalej left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot Bot commented Mar 3, 2026 •

edited

Loading

codecov Bot commented Mar 3, 2026 •

edited

Loading