Skip to content

Commit

Permalink
ci: add windows testing to CI by lifting OS into its own test matrix … (
Browse files Browse the repository at this point in the history
#2081)

Co-authored-by: Michael Brooks <[email protected]>
  • Loading branch information
filmaj and mwbrooks authored Oct 31, 2024
1 parent 8dcdb22 commit 0711cf1
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 16 deletions.
42 changes: 36 additions & 6 deletions .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ on:

jobs:
test:
runs-on: ubuntu-latest
timeout-minutes: 10
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
node-version: [18.x, 20.x, 22.x]
package:
- cli-hooks
Expand All @@ -24,33 +24,63 @@ jobs:
- types
- web-api
- webhook
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
- run: npm --version
- name: Build and Run Tests in Each Package
- run: npm install
working-directory: packages/${{ matrix.package }}
- name: Link dependent packages (*nix)
if: matrix.os == 'ubuntu-latest'
working-directory: packages/${{ matrix.package }}
run: |
npm install
# depending on which package we are testing, also npm link up other dependent packages
# depending on which package we are testing, also npm link up dependent packages within this monorepo
case "$PWD" in
*/webhook) pushd ../types && npm i && popd && npm link ../types;;
*/web-api) pushd ../types && npm i && popd && npm link ../types && pushd ../logger && npm i && popd && npm link ../logger;;
*/oauth) pushd ../logger && npm i && popd && npm link ../logger && pushd ../web-api && npm i && popd && npm link ../web-api;;
*/socket-mode) pushd ../logger && npm i && popd && npm link ../logger && pushd ../web-api && npm i && popd && npm link ../web-api;;
*) ;; # default
esac
npm test
- name: Link dependent packages (Windows)
if: matrix.os == 'windows-latest'
working-directory: packages/${{ matrix.package }}
run: |
# depending on which package we are testing, also npm link up dependent packages within this monorepo
# NOTE: the following is PowerShell
echo "$pwd"
switch -Wildcard ( "$pwd" )
{
'*\webhook'
{
pushd ..\types && npm i && popd && npm link ..\types
}
'*\web-api'
{
pushd ..\types && npm i && popd && npm link ..\types && pushd ..\logger && npm i && popd && npm link ..\logger
}
'*\oauth'
{
pushd ..\logger && npm i && popd && npm link ..\logger && pushd ..\web-api && npm i && popd && npm link ..\web-api
}
'*\socket-mode'
{
pushd ..\logger && npm i && popd && npm link ..\logger && pushd ..\web-api && npm i && popd && npm link ..\web-api
}
}
- run: npm test
working-directory: packages/${{ matrix.package }}
- name: Check for coverage report existence
id: check_coverage
uses: andstor/file-existence-action@v3
with:
files: packages/${{ matrix.package }}/coverage/lcov.info
- name: Upload code coverage
if: matrix.node-version == '22.x' && steps.check_coverage.outputs.files_exists == 'true'
if: matrix.node-version == '22.x' && matrix.os == 'ubuntu-latest' && steps.check_coverage.outputs.files_exists == 'true'
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
10 changes: 6 additions & 4 deletions packages/cli-test/src/cli/commands/platform.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,12 @@ describe('platform commands', () => {
sandbox.stub(shell, 'kill').rejects();
await assert.rejects(platform.runStop({ proc: fakeProcess }));
});
it('should reject if waitForShutdown=true and waitForOutput rejects', async () => {
sandbox.stub(shell, 'kill').resolves();
waitForOutputSpy.rejects();
await assert.rejects(platform.runStop({ proc: fakeProcess, waitForShutdown: true }));
it('non-Windows only: should reject if waitForShutdown=true and waitForOutput rejects', async () => {
if (process.platform !== 'win32') {
sandbox.stub(shell, 'kill').resolves();
waitForOutputSpy.rejects();
await assert.rejects(platform.runStop({ proc: fakeProcess, waitForShutdown: true }));
}
});
it('should resolve immediately if waitForShutdown=false and shell.kill resolve', async () => {
sandbox.stub(shell, 'kill').resolves();
Expand Down
11 changes: 5 additions & 6 deletions packages/web-api/src/WebClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ describe('WebClient', () => {
} catch (e) {
// biome-ignore lint/suspicious/noExplicitAny: TODO: type this better, should be whatever error class web-api throws for timeouts
const error = e as any;
assert.isTrue((logger.warn as sinon.SinonStub).calledOnce, 'expected Logger to be called once');
assert.equal(error.code, ErrorCode.RequestError);
assert.equal(error.original.config.timeout, timeoutOverride);
assert.equal(error.original.isAxiosError, true);
assert.instanceOf(error, Error);
assert.isTrue((logger.warn as sinon.SinonStub).calledOnce, 'expected Logger to be called once');
}
});
});
Expand Down Expand Up @@ -463,14 +463,13 @@ describe('WebClient', () => {
const scope = nock('https://slack.com', {
reqheaders: {
'User-Agent': (value) => {
// User Agent value is different across platforms.
// on mac this is: @slack:web-api/7.7.0 node/18.15.0 darwin/23.6.0
// on windows this is: @slack:web-api/7.7.0 cmd.exe /22.10.0 win32/10.0.20348
const metadata = parseUserAgentIntoMetadata(value);
// NOTE: this assert isn't that strong and doesn't say anything about the values. at this time, there
// isn't a good way to test this without dupicating the logic of the code under test.
assert.containsAllKeys(metadata, ['node', '@slack:web-api']);
// NOTE: there's an assumption that if there's any keys besides these left at all, its the platform part
metadata.node = undefined;
metadata['@slack:client'] = undefined;
assert.isNotEmpty(metadata);
assert.containsAllKeys(metadata, ['@slack:web-api']);
return true;
},
},
Expand Down

0 comments on commit 0711cf1

Please sign in to comment.