Skip to content

Commit

Permalink
refactor: commands into command classes (#424)
Browse files Browse the repository at this point in the history
* fix: quirks with version commands not always handling errors properly

* fix: use the right accessor for getting config data

* refactor: commands into command classes

* fix: typos

* fix: we know the name of the login command

* fix: reverting an unnecessary change

* fix: pr feedback

* fix: bring back snapshot

* fix: pulling some snapshots that have color in them as they're causing flaky tests

* test: tweaks

* fix: removing a dupe color config
  • Loading branch information
erunion authored Jan 12, 2022
1 parent 026c633 commit 7b8b378
Show file tree
Hide file tree
Showing 28 changed files with 959 additions and 907 deletions.
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"] }],

/**
* 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
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 });
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

0 comments on commit 7b8b378

Please sign in to comment.