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

refactor: commands into command classes #424

Merged
merged 12 commits into from
Jan 12, 2022
6 changes: 6 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
"extends": ["@readme/eslint-config"],
"root": true,
"rules": {
/**
* Because our command classes have a `run` method that might not always call `this` we need to
* explicitly exclude `run` from this rule.
*/
"class-methods-use-this": ["error", { "exceptMethods": ["run"] }],
Copy link
Member

Choose a reason for hiding this comment

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

Wanna add a small comment explaining why we need this?


/**
* This is a small rule to prevent us from using console.log() statements in our commands.
*
Expand Down
4 changes: 0 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,3 @@ jobs:

- name: Run tests
run: npm test
env:
# `chalk` has troubles with color detection while on CI.
# https://github.com/chalk/supports-color/issues/106
FORCE_COLOR: 1
5 changes: 3 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules
coverage
node_modules/
coverage/
swagger.json
Copy link
Member

Choose a reason for hiding this comment

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

We technically shouldn’t need this since a successful test run will result in this file being deleted properly… but I suppose doesn’t hurt!

23 changes: 0 additions & 23 deletions __tests__/cmds/__snapshots__/versions.test.js.snap

This file was deleted.

7 changes: 5 additions & 2 deletions __tests__/cmds/docs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ const frontMatter = require('gray-matter');

const APIError = require('../../src/lib/apiError');

const docs = require('../../src/cmds/docs');
const docsEdit = require('../../src/cmds/docs/edit');
const DocsCommand = require('../../src/cmds/docs');
const DocsEditCommand = require('../../src/cmds/docs/edit');

const docs = new DocsCommand();
const docsEdit = new DocsEditCommand();

const fixturesDir = `${__dirname}./../__fixtures__`;
const key = 'API_KEY';
Expand Down
5 changes: 4 additions & 1 deletion __tests__/cmds/login.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
const nock = require('nock');
const config = require('config');
const configStore = require('../../src/lib/configstore');
const cmd = require('../../src/cmds/login');
const Command = require('../../src/cmds/login');
const APIError = require('../../src/lib/apiError');

const cmd = new Command();

const email = '[email protected]';
const password = '123456';
const project = 'subdomain';
Expand Down Expand Up @@ -33,6 +35,7 @@ describe('rdme login', () => {
const mock = nock(config.get('host')).post('/api/v1/login', { email, password, project }).reply(200, { apiKey });

await expect(cmd.run({ email, password, project })).resolves.toMatchSnapshot();

mock.done();

expect(configStore.get('apiKey')).toBe(apiKey);
Expand Down
9 changes: 5 additions & 4 deletions __tests__/cmds/logout.test.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
const config = require('config');
const configStore = require('../../src/lib/configstore');
const cmd = require('../../src/cmds/logout');
const loginCmd = require('../../src/cmds/login');
const Command = require('../../src/cmds/logout');

const cmd = new Command();

describe('rdme logout', () => {
it("should report the user as logged out if they aren't logged in", () => {
configStore.delete('email');
configStore.delete('project');

return expect(cmd.run({})).resolves.toBe(
`You have logged out of ReadMe. Please use \`${config.get('cli')} ${loginCmd.command}\` to login again.`
`You have logged out of ReadMe. Please use \`${config.get('cli')} login\` to login again.`
);
});

Expand All @@ -18,7 +19,7 @@ describe('rdme logout', () => {
configStore.set('project', 'subdomain');

await expect(cmd.run({})).resolves.toBe(
`You have logged out of ReadMe. Please use \`${config.get('cli')} ${loginCmd.command}\` to login again.`
`You have logged out of ReadMe. Please use \`${config.get('cli')} login\` to login again.`
);

expect(configStore.get('email')).toBeUndefined();
Expand Down
7 changes: 4 additions & 3 deletions __tests__/cmds/open.test.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
const chalk = require('chalk');
const config = require('config');
const configStore = require('../../src/lib/configstore');
const cmd = require('../../src/cmds/open');
const loginCmd = require('../../src/cmds/login');
const Command = require('../../src/cmds/open');

const cmd = new Command();

describe('rdme open', () => {
it('should error if no project provided', () => {
configStore.delete('project');

return expect(cmd.run({})).rejects.toThrow(`Please login using \`${config.get('cli')} ${loginCmd.command}\`.`);
return expect(cmd.run({})).rejects.toThrow(`Please login using \`${config.get('cli')} login\`.`);
});

it('should open the project', () => {
Expand Down
7 changes: 5 additions & 2 deletions __tests__/cmds/openapi.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ const chalk = require('chalk');
const config = require('config');
const fs = require('fs');
const promptHandler = require('../../src/lib/prompts');
const swagger = require('../../src/cmds/swagger');
const openapi = require('../../src/cmds/openapi');
const SwaggerCommand = require('../../src/cmds/swagger');
const OpenAPICommand = require('../../src/cmds/openapi');
const APIError = require('../../src/lib/apiError');

const openapi = new OpenAPICommand();
const swagger = new SwaggerCommand();

const key = 'API_KEY';
const id = '5aa0409b7cf527a93bfb44df';
const version = '1.0.0';
Expand Down
4 changes: 3 additions & 1 deletion __tests__/cmds/validate.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const fs = require('fs');
const chalk = require('chalk');
const validate = require('../../src/cmds/validate');
const Command = require('../../src/cmds/validate');

const validate = new Command();

const getCommandOutput = () => {
return [console.info.mock.calls.join('\n\n')].filter(Boolean).join('\n\n');
Expand Down
24 changes: 18 additions & 6 deletions __tests__/cmds/versions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ const config = require('config');
const promptHandler = require('../../src/lib/prompts');
const APIError = require('../../src/lib/apiError');

const versions = require('../../src/cmds/versions');
const createVersion = require('../../src/cmds/versions/create');
const deleteVersion = require('../../src/cmds/versions/delete');
const updateVersion = require('../../src/cmds/versions/update');
const VersionsCommand = require('../../src/cmds/versions');
const CreateVersionCommand = require('../../src/cmds/versions/create');
const DeleteVersionCommand = require('../../src/cmds/versions/delete');
const UpdateVersionCommand = require('../../src/cmds/versions/update');

const key = 'API_KEY';
const version = '1.0.0';
Expand Down Expand Up @@ -40,6 +40,8 @@ describe('rdme versions*', () => {
afterEach(() => nock.cleanAll());

describe('rdme versions', () => {
const versions = new VersionsCommand();

it('should error if no api key provided', () => {
return expect(versions.run({})).rejects.toStrictEqual(
new Error('No project API key provided. Please use `--key`.')
Expand All @@ -52,7 +54,9 @@ describe('rdme versions*', () => {
.basicAuth({ user: key })
.reply(200, [versionPayload, version2Payload]);

await expect(versions.run({ key })).resolves.toMatchSnapshot();
const table = await versions.run({ key });
Copy link
Member Author

@erunion erunion Jan 11, 2022

Choose a reason for hiding this comment

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

Reverted this back to using these two assertions because the FORCE_COLOR env variable doesn't seem to always get used within the colors dependency of cli-table.

expect(table).toContain(version);
expect(table).toContain(version2);
mockRequest.done();
});

Expand All @@ -73,7 +77,9 @@ describe('rdme versions*', () => {
.basicAuth({ user: key })
.reply(200, versionPayload);

await expect(versions.run({ key, version })).resolves.toMatchSnapshot();
const table = await versions.run({ key, version });
expect(table).toContain(version);
expect(table).not.toContain(version2);
mockRequest.done();
});

Expand All @@ -90,6 +96,8 @@ describe('rdme versions*', () => {
});

describe('rdme versions:create', () => {
const createVersion = new CreateVersionCommand();

it('should error if no api key provided', () => {
return createVersion.run({}).catch(err => {
expect(err.message).toBe('No project API key provided. Please use `--key`.');
Expand Down Expand Up @@ -143,6 +151,8 @@ describe('rdme versions*', () => {
});

describe('rdme versions:delete', () => {
const deleteVersion = new DeleteVersionCommand();

it('should error if no api key provided', () => {
return expect(deleteVersion.run({})).rejects.toStrictEqual(
new Error('No project API key provided. Please use `--key`.')
Expand Down Expand Up @@ -185,6 +195,8 @@ describe('rdme versions*', () => {
});

describe('rdme versions:update', () => {
const updateVersion = new UpdateVersionCommand();

it('should error if no api key provided', () => {
return expect(updateVersion.run({})).rejects.toStrictEqual(
new Error('No project API key provided. Please use `--key`.')
Expand Down
9 changes: 4 additions & 5 deletions __tests__/cmds/whoami.test.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
const config = require('config');
const configStore = require('../../src/lib/configstore');
const cmd = require('../../src/cmds/whoami');
const loginCmd = require('../../src/cmds/login');
const Command = require('../../src/cmds/whoami');

const cmd = new Command();

describe('rdme whoami', () => {
it('should error if user is not authenticated', () => {
configStore.delete('email');
configStore.delete('project');

return expect(cmd.run({})).rejects.toStrictEqual(
new Error(`Please login using \`${config.get('cli')} ${loginCmd.command}\`.`)
);
return expect(cmd.run({})).rejects.toStrictEqual(new Error(`Please login using \`${config.get('cli')} login\`.`));
});

it('should return the authenticated user', () => {
Expand Down
4 changes: 3 additions & 1 deletion __tests__/set-node-env.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Chalk has trouble with Jest sometimes in test snapshots so we're disabling colorization here for all tests.
// The `chalk` and `colors` libraries have trouble with Jest sometimes in test snapshots so we're disabling
// colorization here for all tests.
// https://github.com/chalk/supports-color/issues/106
process.env.FORCE_COLOR = 0;

process.env.NODE_ENV = 'testing';
Loading