Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/chatty-tires-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@slack/oauth": patch
---

fix: write files to the file installation store using the path.join method
2 changes: 2 additions & 0 deletions .claude/settings.json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤩

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"Bash(git show:*)",
"Bash(git status:*)",
"Bash(grep:*)",
"Bash(head:*)",
"Bash(ls:*)",
"Bash(node --version:*)",
"Bash(npm --version:*)",
Expand All @@ -27,6 +28,7 @@
"Bash(npm run lint:*)",
"Bash(npm run lint:fix:*)",
"Bash(npm test:*)",
"Bash(tail:*)",
"Bash(tree:*)",
"WebFetch(domain:docs.slack.dev)",
"WebFetch(domain:github.com)",
Expand Down
3 changes: 2 additions & 1 deletion .github/maintainers_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ The Node SDK is made up of multiple, individual packages, each with their own te
```sh
npm install
npm run lint
npm test
npm test --workspace packages/web-api
```

This project has tests for individual packages as `*.spec.js` files and inside of each package's `src` directory. Also, for verifying the behavior with the real Slack server-side and developer experience with installed packages, you can run the tests amd scripts under `prod-server-integration-tests`. Refer to the README file in the directory for details. These tests are supposed to be run in the project maintainers' manual execution. They are not part of CI builds for now.
This project has tests for individual packages as `*.test.ts` (or `*.test.js`) files inside of each package's `src` directory. Tests use `node:test` as the test runner and `node:assert/strict` for assertions. Also, for verifying the behavior with the real Slack server-side and developer experience with installed packages, you can run the tests and scripts under `prod-server-integration-tests`. Refer to the README file in the directory for details. These tests are supposed to be run in the project maintainers' manual execution. They are not part of CI builds for now.

Upon opening a PR, tests are executed by GitHub Actions, our continuous integration system. GitHub Actions runs several, more granular builds in order to report on success and failure in a more targeted way.

Expand Down
61 changes: 30 additions & 31 deletions .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,17 @@ on:

jobs:
test:
timeout-minutes: 4
timeout-minutes: 6
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
node-version: [18.x, 20.x, 22.x]
package:
- cli-hooks
- cli-test
- logger
- oauth
- rtm-api
- socket-mode
- types
- web-api
- webhook
os:
- "ubuntu-latest"
- "windows-latest"
node-version:
- "18.x"
- "20.x"
- "22.x"
runs-on: ${{ matrix.os }}
permissions:
contents: read
Expand All @@ -40,10 +35,12 @@ jobs:
uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0
with:
node-version: ${{ matrix.node-version }}
- run: npm --version
- name: Install dependencies
- name: Check versions
run: |
npm install --verbose
node --version
npm --version
- name: Install dependencies
run: npm install --verbose
- name: Build packages
run: |
# Build packages without internal dependencies
Expand All @@ -62,32 +59,34 @@ jobs:
npm run build --workspace=@slack/oauth
npm run build --workspace=@slack/rtm-api
npm run build --workspace=@slack/socket-mode
- name: Run tests
run: |
npm run lint
npm test --workspace=@slack/${{ matrix.package }}
- name: Check for coverage report existence
id: check_coverage
uses: andstor/file-existence-action@076e0072799f4942c8bc574a82233e1e4d13e9d6 # v3.0.0
with:
files: packages/${{ matrix.package }}/coverage/lcov.info
- name: Lint
run: npm run lint
- name: Run tests (Node 18/20)
if: matrix.node-version != '22.x'
shell: bash
# 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'
shell: bash
run: npm test
- name: Upload code coverage
if: matrix.node-version == '22.x' && matrix.os == 'ubuntu-latest' && steps.check_coverage.outputs.files_exists == 'true'
if: matrix.node-version == '22.x' && matrix.os == 'ubuntu-latest'
uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2
with:
directory: packages/${{ matrix.package }}/coverage
fail_ci_if_error: true
flags: ${{ matrix.package }}
files: packages/*/lcov.info
flags: cli-hooks,cli-test,logger,oauth,socket-mode,web-api,webhook
report_type: coverage
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true
- name: Upload test results to Codecov
- name: Upload test results
if: ${{ !cancelled() }}
uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2
with:
fail_ci_if_error: true
files: packages/${{ matrix.package }}/coverage/test-results.xml
flags: ${{ matrix.node-version }},${{ matrix.os }},${{ matrix.package }}
files: packages/cli-hooks/test-results.xml,packages/cli-test/test-results.xml,packages/logger/test-results.xml,packages/oauth/test-results.xml,packages/socket-mode/test-results.xml,packages/web-api/test-results.xml,packages/webhook/test-results.xml
flags: ${{ matrix.node-version }},${{ matrix.os }}
report_type: test_results
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ scripts/package-lock.json
examples/*/package-lock.json

npm-debug.log
lerna-debug.log
lcov.info
test-results.xml
tmp/
.env*

Expand All @@ -13,4 +14,3 @@ tmp/
*.bk

.DS_Store
.nyc_output/
10 changes: 7 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ npm run build --workspace=@slack/socket-mode
## Code Conventions

- **Formatting**: configured in `biome.json`
- **Test files**: `*.spec.ts` using Mocha + chai + sinon; coverage via c8
- **Test files**: `*.test.{ts,js}` using `node:test` + `node:assert/strict`; coverage via `--experimental-test-coverage`
- **Type tests**: `*.test-d.ts` using tsd
- **Naming conventions for request/response types**:
- Request: `{Namespace}{Action}Arguments` (e.g., `ChatPostMessageArguments`)
Expand Down Expand Up @@ -277,10 +277,14 @@ Entry point for response type generation. It:

## Testing

- **Unit tests**: Mocha + chai + sinon (`*.spec.ts` files), coverage via c8
- **Test framework**: `node:test` (built-in) with `node:assert/strict` for assertions
- **Test runner**: `node --experimental-test-coverage --import tsx --test`
- **Unit tests**: `*.test.{ts,js}` files alongside source (e.g., `src/WebClient.test.ts`)
- **Type tests**: tsd (`*.test-d.ts` files in `packages/web-api/test/types/`)
- **Integration tests**: CommonJS, ESM, and TypeScript compatibility checks
- **CI matrix**: Node 18.x, 20.x, 22.x on Ubuntu + Windows
- **CI matrix**: Node 18.x, 20.x, 22.x on Ubuntu + Windows (6 jobs total)
- **Coverage output**: `lcov.info` at each package root (not in `coverage/` dir)
- **Test results**: `test-results.xml` at each package root (JUnit format)

Per-package test commands:

Expand Down
4 changes: 4 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ flags:
paths:
- packages/cli-hooks/**
carryforward: true
cli-test:
paths:
- packages/cli-test/**
carryforward: true
logger:
paths:
- packages/logger/**
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"scripts": {
"changeset": "npx @changesets/cli",
"lint": "npx @biomejs/biome check packages",
"lint:fix": "npx @biomejs/biome check --write packages"
"lint:fix": "npx @biomejs/biome check --write packages",
"test": "npm test --workspaces --if-present"
},
"devDependencies": {
"@biomejs/biome": "^2.0.5",
Expand Down
3 changes: 0 additions & 3 deletions packages/cli-hooks/.gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# Node and NPM stuff
/node_modules
package-lock.json

# Coverage carryover
/coverage
10 changes: 3 additions & 7 deletions packages/cli-hooks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@
},
"scripts": {
"build": "shx chmod +x src/*.js",
"prebuild": "shx rm -rf ./coverage",
"prelint": "tsc --noemit --module es2022 --maxNodeModuleJsDepth 0 --project ./jsconfig.json",
"test": "c8 --config ./test/.c8rc.json mocha --config ./test/.mocharc.json src/*.spec.js"
"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"
},
"bin": {
"slack-cli-check-update": "src/check-update.js",
Expand All @@ -53,14 +54,9 @@
},
"devDependencies": {
"@types/minimist": "^1.2.5",
"@types/mocha": "^10.0.6",
"@types/node": "^25.0.3",
"@types/semver": "^7.5.6",
"@types/sinon": "^21.0.0",
"c8": "^10.1.2",
"mocha": "^11.0.1",
"mocha-junit-reporter": "^2.2.1",
"mocha-multi-reporters": "^1.5.1",
"shx": "^0.4.0",
"sinon": "^21.0.0",
"typescript": "5.9.3"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import assert from 'node:assert';
import childProcess from 'node:child_process';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import util from 'node:util';
import { after, before, describe, it } from 'mocha';
import { after, before, describe, it } from 'node:test';
import sinon from 'sinon';

import checkForSDKUpdates, {
Expand Down Expand Up @@ -47,34 +48,23 @@ function mockNPM(command) {

describe('check-update implementation', async () => {
describe('collects recent package versions', async () => {
const tempDir = path.join(process.cwd(), 'tmp');
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-update-'));
Comment on lines -50 to +51
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

const packageJSONFilePath = path.join(tempDir, 'package.json');

before(() => {
sinon.stub(util, 'promisify').returns((/** @type {string} */ command) => {
const info = mockNPM(command);
return Promise.resolve({ stdout: info });
sinon.stub(childProcess, 'exec').callsFake((command, cb) => {
cb(null, { stdout: mockNPM(command), stderr: '' });
});
Comment on lines +55 to 57
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📺 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!

if (!fs.existsSync(tempDir)) {
fs.mkdirSync(tempDir);
}
if (!fs.existsSync(packageJSONFilePath)) {
fs.writeFileSync(packageJSONFilePath, JSON.stringify(packageJSON, null, 2));
}
fs.writeFileSync(packageJSONFilePath, JSON.stringify(packageJSON, null, 2));
});

after(() => {
sinon.restore();
if (fs.existsSync(packageJSONFilePath)) {
fs.unlinkSync(packageJSONFilePath);
}
if (fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true });
}
fs.rmSync(tempDir, { recursive: true, force: true });
});

it('shows version information for packages', async () => {
const updates = await checkForSDKUpdates('./tmp');
const updates = await checkForSDKUpdates(tempDir);
const expected = {
name: 'the Slack SDK',
error: undefined,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert';
import { describe, it } from 'mocha';
import { describe, it } from 'node:test';

import doctor from './doctor.js';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert';
import { describe, it } from 'mocha';
import { describe, it } from 'node:test';

import getHooks from './get-hooks.js';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import assert from 'node:assert';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { after, before, describe, it } from 'mocha';
import { after, before, describe, it } from 'node:test';

import getManifestData from './get-manifest.js';

Expand All @@ -21,28 +22,20 @@ describe('get-manifest implementation', async () => {
});

describe('broken project manifest file exists', async () => {
const tempDir = path.join(process.cwd(), 'tmp');
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'manifest-broken-'));
const filePath = path.join(tempDir, 'manifest.json');

before(() => {
if (!fs.existsSync(tempDir)) {
fs.mkdirSync(tempDir);
}
fs.writeFileSync(filePath, '{');
});

after(() => {
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
if (fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true });
}
fs.rmSync(tempDir, { recursive: true, force: true });
});

it('should error for invalid manifest.json', async () => {
try {
getManifestData(process.cwd());
getManifestData(tempDir);
} catch (err) {
if (err instanceof Error) {
const nodeV18Error = 'SyntaxError: Unexpected end of JSON input';
Expand All @@ -57,7 +50,7 @@ describe('get-manifest implementation', async () => {
});

describe('contains project manifest file', async () => {
const tempDir = path.join(process.cwd(), 'tmp');
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'manifest-valid-'));
const filePath = path.join(tempDir, 'manifest.json');
const manifest = {
display_information: {
Expand All @@ -77,24 +70,16 @@ describe('get-manifest implementation', async () => {
};

before(() => {
if (!fs.existsSync(tempDir)) {
fs.mkdirSync(tempDir);
}
fs.writeFileSync(filePath, JSON.stringify(manifest, null, 2));
});

after(() => {
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
if (fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true });
}
fs.rmSync(tempDir, { recursive: true, force: true });
});

it('should return existing manifest values', async () => {
try {
const parsedManifest = getManifestData(process.cwd());
const parsedManifest = getManifestData(tempDir);
assert.deepEqual(manifest, parsedManifest);
} catch (err) {
console.error(err);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert';
import { afterEach, beforeEach, describe, it } from 'mocha';
import { afterEach, beforeEach, describe, it } from 'node:test';
import sinon from 'sinon';

import { DefaultProtocol, getProtocol, MessageBoundaryProtocol } from './protocols.js';
Expand Down
Loading