Skip to content
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

web-api: basic module system integration tests, removing unused bits, dependency updates #1724

Merged
merged 5 commits into from Jan 16, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion LICENSE
@@ -1,6 +1,6 @@
MIT License

Copyright (c) 2014-2020 Slack Technologies, LLC
Copyright (c) 2014- Slack Technologies, LLC

Permission is hereby granted, free of charge, to any person obtaining
a copy of this software and associated documentation files (the
Expand Down
3 changes: 0 additions & 3 deletions packages/web-api/deno-shims/buffer-shim.js
Copy link
Member

Choose a reason for hiding this comment

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

Farewell, deno-shims/buffer-shim.js 👋 😔

This file was deleted.

1 change: 0 additions & 1 deletion packages/web-api/deno-shims/qs-shim.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/web-api/deno-shims/util-shim.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/web-api/deno-shims/xhr-shim.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/web-api/deno-shims/zlib-shim.js

This file was deleted.

54 changes: 27 additions & 27 deletions packages/web-api/package.json
Expand Up @@ -38,7 +38,8 @@
"build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output",
"lint": "eslint --ext .ts src",
"mocha": "mocha --config .mocharc.json src/*.spec.js",
"test": "npm run lint && npm run test:unit && npm run test:types",
"test": "npm run lint && npm run test:unit && npm run test:types && npm run test:integration",
"test:integration": "npm run build && node test/integration/commonjs-project/index.js && node test/integration/esm-project/index.mjs",
"test:unit": "npm run build && nyc --reporter=text-summary npm run mocha",
"test:types": "tsd",
"ref-docs:model": "api-extractor run",
Expand All @@ -52,37 +53,36 @@
"eventemitter3": "^5.0.1",
"form-data": "^4.0.0",
"is-electron": "2.2.2",
"is-stream": "^2.0.0",
"p-queue": "^6.6.2",
"p-retry": "^4.6.1",
"is-stream": "^2",
"p-queue": "^6",
"p-retry": "^4",
"retry": "^0.13.1"
},
"devDependencies": {
"@aoberoi/capture-console": "^1.1.0",
"@microsoft/api-extractor": "^7.38.0",
"@types/chai": "^4.3.5",
"@types/mocha": "^10.0.1",
"@types/sinon": "^10.0.20",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.0",
"busboy": "^1.6.0",
"chai": "^4.3.8",
"eslint": "^8.47.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jsdoc": "^46.5.0",
"eslint-plugin-node": "^11.1.0",
"mocha": "^10.2.0",
"nock": "^13.3.3",
"nyc": "^15.1.0",
"shelljs": "^0.8.3",
"@microsoft/api-extractor": "^7",
"@tsconfig/recommended": "^1",
"@types/chai": "^4",
"@types/mocha": "^10",
"@types/sinon": "^17",
"@typescript-eslint/eslint-plugin": "^6",
"@typescript-eslint/parser": "^6",
"busboy": "^1",
"chai": "^4",
"eslint": "^8",
"eslint-config-airbnb-base": "^15",
"eslint-config-airbnb-typescript": "^17",
"eslint-plugin-import": "^2",
"eslint-plugin-jsdoc": "^48",
"eslint-plugin-node": "^11",
"mocha": "^10",
"nock": "^13",
"nyc": "^15",
"shx": "^0.3.2",
"sinon": "^15.2.0",
"sinon": "^17",
"source-map-support": "^0.5.21",
"ts-node": "^10.8.1",
"tsd": "0.29.0",
"typescript": "^4.1"
"ts-node": "^10",
"tsd": "^0.30.0",
"typescript": "5.3.3"
Copy link
Member

Choose a reason for hiding this comment

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

🔒🔒🔒🔒🔒.3.3 👍 Happy to remove breaking builds from this!

},
"tsd": {
"directory": "test/types"
Expand Down
33 changes: 16 additions & 17 deletions packages/web-api/src/WebClient.spec.js
Expand Up @@ -9,7 +9,6 @@ const { LogLevel } = require('./logger');
const { addAppMetadata } = require('./instrument');
const { rapidRetryPolicy } = require('./retry-policies');
const { Methods } = require('./methods');
const { CaptureConsole } = require('@aoberoi/capture-console');
const nock = require('nock');
const Busboy = require('busboy');
const sinon = require('sinon');
Expand Down Expand Up @@ -61,24 +60,24 @@ describe('WebClient', function () {
});

describe('has an option to change the log output severity', function () {
let sandbox = null;
beforeEach(function () {
this.capture = new CaptureConsole();
this.capture.startCapture();
sandbox = sinon.createSandbox();
sandbox.stub(console, 'debug');
});
afterEach(function () {
sandbox.restore();
});
it('outputs a debug log on initialization', function () {
const debuggingClient = new WebClient(token, { logLevel: LogLevel.DEBUG });
const output = this.capture.getCapturedText();
const output = console.debug.getCalls()[0].args.join(' ');
assert.isNotEmpty(output); // should have at least 1 log line, but not asserting since that is an implementation detail
});
afterEach(function () {
this.capture.stopCapture();
});
});

describe('has a logger option', function () {
let sandbox = null;
beforeEach(function () {
this.capture = new CaptureConsole();
this.capture.startCapture();
this.logger = {
debug: sinon.spy(),
info: sinon.spy(),
Expand All @@ -87,21 +86,22 @@ describe('WebClient', function () {
setLevel: sinon.spy(),
setName: sinon.spy(),
};
sandbox = sinon.createSandbox();
sandbox.stub(console, 'debug');
});
afterEach(function () {
sandbox.restore();
});
it('sends logs to a logger and not to stdout', function () {
const debuggingClient = new WebClient(token, { logLevel: LogLevel.DEBUG, logger: this.logger });
assert.isTrue(this.logger.debug.called);
const capturedOutput = this.capture.getCapturedText();
assert.isEmpty(capturedOutput);
assert.isEmpty(console.debug.getCalls());
});
it('never modifies the original logger', function () {
new WebClient(token, { logger: this.logger });
// Calling #setName of the given logger is destructive
assert.isFalse(this.logger.setName.called);
});
afterEach(function () {
this.capture.stopCapture();
});
});

describe('has an option to override the Axios timeout value', function () {
Expand Down Expand Up @@ -558,17 +558,16 @@ describe('WebClient', function () {
});
});

// TODO: Upgrading busboy to 1.x breaks these tests
describe('when API arguments contain binary to upload', function () {
beforeEach(function () {
const self = this;
self.scope = nock('https://slack.com')
.post(/api/)
// rather than matching on the body, that nock cannot do for content-type multipart/form-data, we use the
// response to signal that the body was correctly serialized
.reply(function (uri, requestBody, cb) {
.reply(function (_uri, requestBody, cb) {
// busboy is a parser for for multipart/form-data bodies
const busboy = new Busboy({ headers: this.req.headers });
const busboy = Busboy({ headers: this.req.headers });
// capture state about all the parts that are in the body
const parts = { files: [], fields: [], errors: [] };

Expand Down
4 changes: 2 additions & 2 deletions packages/web-api/src/WebClient.ts
Expand Up @@ -578,8 +578,8 @@ export class WebClient extends Methods {
throw error;
}
});

return pRetry(task, this.retryConfig);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return pRetry(task, this.retryConfig) as Promise<AxiosResponse<any, any>>;
}

/**
Expand Down
7 changes: 0 additions & 7 deletions packages/web-api/src/file-upload.ts
Expand Up @@ -4,13 +4,6 @@ import { Logger } from '@slack/logger';
import { errorWithCode, ErrorCode } from './errors';
import { FilesCompleteUploadExternalArguments, FilesUploadV2Arguments, FileUploadBinaryContents, FileUploadStringContents, FileUploadV2, FileUploadV2Job } from './types/request/files';

/**
* Returns a fileUploadJob used to represent the of the file upload job and
* required metadata.
* @param options Options provided by user
* @param channelId optional channel id to share file with, omitted, channel is private
* @returns
*/
export async function getFileUploadJob(
options: FilesUploadV2Arguments | FileUploadV2,
logger: Logger,
Expand Down
2 changes: 1 addition & 1 deletion packages/web-api/src/index.ts
Expand Up @@ -30,4 +30,4 @@ export { addAppMetadata } from './instrument';
export * from './methods';
export { default as Method } from './methods';

export * from './types/response';
export * from './types/response/index';
2 changes: 1 addition & 1 deletion packages/web-api/src/methods.ts
Expand Up @@ -236,7 +236,7 @@ import type {
AdminWorkflowsPermissionsLookupResponse,
AdminAppsConfigLookupResponse,
AdminAppsConfigSetResponse,
} from './types/response';
} from './types/response/index';
// Request types
import type { WorkflowsStepCompletedArguments, WorkflowsStepFailedArguments, WorkflowsUpdateStepArguments } from './types/request/workflows';
import type { ViewsUpdateArguments, ViewsOpenArguments, ViewsPushArguments, ViewsPublishArguments } from './types/request/views';
Expand Down
2 changes: 1 addition & 1 deletion packages/web-api/src/types/request/files.ts
@@ -1,6 +1,6 @@
import type { Stream } from 'node:stream';
import type { CursorPaginationEnabled, OptionalTeamAssignable, TokenOverridable, TraditionalPagingEnabled } from './common';
import type { FilesGetUploadURLExternalResponse } from '../response';
import type { FilesGetUploadURLExternalResponse } from '../response/index';

interface FileArgument {
/** @description Encoded file ID. */
Expand Down
11 changes: 11 additions & 0 deletions packages/web-api/test/integration/commonjs-project/index.js
@@ -0,0 +1,11 @@
/* eslint-disable */
const WebApi = require('../../../dist/index');
const client = new WebApi.WebClient('invalid-token');
const assert = require('assert');
client.auth.test().then((res) => {
console.error('❌ Unexpected `auth.test` API success! Exiting CJS project integration test with non-zero exit code.');
process.exit(1);
}).catch((e) => {
assert(e.message.includes('invalid_auth'), '❌ Did not receive expected "invalid_auth" response from `auth.test` API, CJS project integration test failed.');
console.log('✅ CJS project integration test succeeded!');
});
12 changes: 12 additions & 0 deletions packages/web-api/test/integration/commonjs-project/package.json
@@ -0,0 +1,12 @@
{
"name": "commonjs-project",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC"
}
12 changes: 12 additions & 0 deletions packages/web-api/test/integration/esm-project/index.mjs
@@ -0,0 +1,12 @@
/* eslint-disable */
import { WebClient } from '../../../dist/index.js';
import assert from 'assert';
const client = new WebClient('invalid-token');
try {
const res = await client.auth.test();
console.error('❌ Unexpected auth.test success! Exiting ESM project integration test with non-zero exit code.');
process.exit(1);
} catch (e) {
assert(e.message.includes('invalid_auth'), '❌ Did not receive expected "invalid_auth" response from `auth.test` API, ESM project integration test failed.');
console.log('✅ ESM project integration test succeeded!');
Copy link
Member

Choose a reason for hiding this comment

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

}
13 changes: 13 additions & 0 deletions packages/web-api/test/integration/esm-project/package.json
@@ -0,0 +1,13 @@
{
"name": "esm-project",
"type": "module",
"version": "1.0.0",
"description": "",
"main": "index.mjs",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC"
}
36 changes: 12 additions & 24 deletions packages/web-api/tsconfig.json
@@ -1,39 +1,27 @@
{
"$schema": "https://json.schemastore.org/tsconfig",
"compilerOptions": {
"target": "es2017",
"module": "commonjs",
"declaration": true,
"declarationMap": true,
"sourceMap": true,
"outDir": "dist",

"strict": true,
"noFallthroughCasesInSwitch": true,
"noImplicitReturns": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,

"moduleResolution": "node",
"baseUrl": ".",
"paths": {
"*": ["./types/*"]
},
"esModuleInterop" : true,

// Not using this setting because its only used to require the package.json file, and that would change the
// structure of the files in the dist directory because package.json is not located inside src. It would be nice
// to use import instead of require(), but its not worth the tradeoff of restructuring the build (for now).
// "resolveJsonModule": true,
"outDir": "dist",
"sourceMap": true,
},
"extends": "@tsconfig/recommended/tsconfig.json",
Copy link
Member

Choose a reason for hiding this comment

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

🙌 🙌

"include": [
"src/**/*"
],
"exclude": [
"src/**/*.spec.js",
"src/**/*.js"
],
"jsdoc": {
"out": "support/jsdoc",
"access": "public"
},
"exclude": [
"src/**/*.js"
],
"ts-node": {
"esm": true
}
}