Skip to content

Commit

Permalink
refactor: migrate from enquirer to prompts (#570)
Browse files Browse the repository at this point in the history 
* fix: throw errors if version POST fails

found an interesting bug... if someone creates a version but the request is invalid for any reason, we just swallow the request and call it a day 😬

This slightly refactors the logic so we actually digest the request and then use the data from its response, rather than the raw input from the user.

* chore: small refactor to align patterns

* feat: first pass at migration version selection

... over to prompts library

this also entailed removing some unnecessary types

* refactor: wrap prompts in exit handler

* temp: why isn't this working 🧐

I've isolated a test here to demonstrate some weirdness going on... basically the generatePrompts() function isn't being called from this test at all, but it works fine when I run the command locally

* Revert "temp: why isn't this working 🧐"

This reverts commit 9c529f6.

* fix: properly add onState function

Since `questions` can be either an object or an array, we have to be a little smarter about how we tack on the `onState` function

* test: stricter test for testing version prompt

I'm so mad lol I hate enquirer (also tysm @erunion for finding this stupid mock that was ruining everything)

* test: update version selection tests

* feat: migrate oas selection to prompts lib

I continue to be so mad at how nice prompts is

* test: more resilient version selection tests

* feat: API definition id in description

* chore: small logic removal

our openapi test coverage is officially 💯

i literally have no idea how to even get to this point in the logic

* refactor: better return types

I think this is better? VSCode seems to flag stuff better this way...

* refactor: consolidate version opts

* docs: add JSDoc for getVersionArg

* refactor(version): consolidate opt types, stricter

jeez, this one was wild.

i noticed we had a lot of duplicate type definitions, so i consolidated all of them in the `versions:create` command. easy enough right?

well I noticed we had union boolean/string definitions for `isPublic`, `main`, etc. even though the opts were only being created as strings—turns out we only had them defined as booleans because of our paltry tests.

I typed those opts as string enums and separated them out into which command they're being used for so it's a little bit more clear.

* refactor: migrate `createVersionPrompt` to prompts

* test: far better tests for prompts

honestly I don't think these were working properly before at all... but now they are!

* test(version:create): far better tests

* fix: correct type

this was holdover from a bad `createVersionPrompt` test. this should always be a boolean.

* chore: clearer question

* fix: document some missing parameters

Also as part of this, I'm moving the `newVersion` param to only live in the `VersionUpdateOptions` type since that's technically only an opt for that command.

Also doing some slight rearranging of the opts in our object destructuring so it aligns with the order of the opt definitions.

* test(version:update): overhauling test suite

god these commands are a mess

* chore: remove redundant catch

* test(versions:create): more coverage

* fix(createVersionPrompt): validation

* test: add TODO

* chore(deps): rip out enquirer 🚮

* feat(login): use prompts

as part of this, i swapped out read for enquirer and isemail for validator

* chore: shorten debug command

* feat: cuter goodbye 👋

* chore: PR feedback

Co-Authored-By: Jon Ursenbach <[email protected]>

* docs: add command attribute descriptions

Co-authored-by: Jon Ursenbach <[email protected]>
  • Loading branch information
@kanadgupta @erunion
kanadgupta and erunion authored Aug 19, 2022
1 parent 0057dab commit 8714967
Show file tree
Hide file tree
Showing 19 changed files with 546 additions and 450 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ npm run build
If you need to debug commands quicker and re-building TS everytime is becoming cumbersome, you can use the debug command, like so:

```sh
npm run debug:bin -- validate __tests__/__fixtures__/ref-oas/petstore.json
npm run debug -- validate __tests__/__fixtures__/ref-oas/petstore.json
```

## Running GitHub Actions Locally 🐳
Expand Down
2 changes: 2 additions & 0 deletions __tests__/cmds/__snapshots__/login.test.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@

exports[`rdme login should post to /login on the API 1`] = `"Successfully logged in as [email protected] to the subdomain project."`;

exports[`rdme login should post to /login on the API if passing in project via opt 1`] = `"Successfully logged in as [email protected] to the subdomain project."`;

exports[`rdme login should send 2fa token if provided 1`] = `"Successfully logged in as [email protected] to the subdomain project."`;
37 changes: 29 additions & 8 deletions __tests__/cmds/login.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import nock from 'nock';
import prompts from 'prompts';

import Command from '../../src/cmds/login';
import APIError from '../../src/lib/apiError';
Expand All @@ -19,23 +20,40 @@ describe('rdme login', () => {
afterEach(() => configStore.clear());

it('should error if no project provided', () => {
prompts.inject([email, password]);
return expect(cmd.run({})).rejects.toStrictEqual(
new Error('No project subdomain provided. Please use `--project`.')
);
});

it('should error if email is invalid', () => {
return expect(cmd.run({ project: 'subdomain', email: 'this-is-not-an-email' })).rejects.toStrictEqual(
new Error('You must provide a valid email address.')
);
prompts.inject(['this-is-not-an-email', password, project]);
return expect(cmd.run({})).rejects.toStrictEqual(new Error('You must provide a valid email address.'));
});

it('should post to /login on the API', async () => {
prompts.inject([email, password, project]);
const apiKey = 'abcdefg';

const mock = getAPIMock().post('/api/v1/login').reply(200, { apiKey });

await expect(cmd.run({})).resolves.toMatchSnapshot();

mock.done();

expect(configStore.get('apiKey')).toBe(apiKey);
expect(configStore.get('email')).toBe(email);
expect(configStore.get('project')).toBe(project);
configStore.clear();
});

it('should post to /login on the API if passing in project via opt', async () => {
prompts.inject([email, password]);
const apiKey = 'abcdefg';

const mock = getAPIMock().post('/api/v1/login', { email, password, project }).reply(200, { apiKey });
const mock = getAPIMock().post('/api/v1/login').reply(200, { apiKey });

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

mock.done();

Expand All @@ -46,6 +64,7 @@ describe('rdme login', () => {
});

it('should error if invalid credentials are given', async () => {
prompts.inject([email, password, project]);
const errorResponse = {
error: 'LOGIN_INVALID',
message: 'Either your email address or password is incorrect',
Expand All @@ -55,11 +74,12 @@ describe('rdme login', () => {

const mock = getAPIMock().post('/api/v1/login', { email, password, project }).reply(401, errorResponse);

await expect(cmd.run({ email, password, project })).rejects.toStrictEqual(new APIError(errorResponse));
await expect(cmd.run({})).rejects.toStrictEqual(new APIError(errorResponse));
mock.done();
});

it('should error if missing two factor token', async () => {
prompts.inject([email, password, project]);
const errorResponse = {
error: 'LOGIN_TWOFACTOR',
message: 'You must provide a two-factor code',
Expand All @@ -69,16 +89,17 @@ describe('rdme login', () => {

const mock = getAPIMock().post('/api/v1/login', { email, password, project }).reply(401, errorResponse);

await expect(cmd.run({ email, password, project })).rejects.toStrictEqual(new APIError(errorResponse));
await expect(cmd.run({})).rejects.toStrictEqual(new APIError(errorResponse));
mock.done();
});

it('should send 2fa token if provided', async () => {
const token = '123456';
prompts.inject([email, password, project, token]);

const mock = getAPIMock().post('/api/v1/login', { email, password, project, token }).reply(200, { apiKey: '123' });

await expect(cmd.run({ email, password, project, token })).resolves.toMatchSnapshot();
await expect(cmd.run({ '2fa': true })).resolves.toMatchSnapshot();
mock.done();
});

Expand Down
79 changes: 68 additions & 11 deletions __tests__/cmds/openapi.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@
import chalk from 'chalk';
import config from 'config';
import nock from 'nock';
import prompts from 'prompts';

import OpenAPICommand from '../../src/cmds/openapi';
import SwaggerCommand from '../../src/cmds/swagger';
import APIError from '../../src/lib/apiError';
import getAPIMock from '../helpers/get-api-mock';

// eslint-disable-next-line @typescript-eslint/no-var-requires
const promptHandler = require('../../src/lib/prompts');

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

Expand Down Expand Up @@ -43,8 +41,6 @@ const successfulUpdate = (specPath, specType = 'OpenAPI') =>

const testWorkingDir = process.cwd();

jest.mock('../../src/lib/prompts');

const getCommandOutput = () => {
return [consoleWarnSpy.mock.calls.join('\n\n'), consoleInfoSpy.mock.calls.join('\n\n')].filter(Boolean).join('\n\n');
};
Expand Down Expand Up @@ -142,7 +138,36 @@ describe('rdme openapi', () => {
return mock.done();
});

it.todo('should test spec selection prompts');
it('should create a new spec via prompts', async () => {
prompts.inject(['create']);
const registryUUID = getRandomRegistryId();

const mock = getAPIMock()
.get(`/api/v1/version/${version}`)
.basicAuth({ user: key })
.reply(200, [{ version }])
.post('/api/v1/api-registry', body => body.match('form-data; name="spec"'))
.reply(201, { registryUUID, spec: { openapi: '3.0.0' } })
.get('/api/v1/api-specification')
.basicAuth({ user: key })
.reply(200, [{ _id: 'spec1', title: 'spec1_title' }])
.post('/api/v1/api-specification', { registryUUID })
.delayConnection(1000)
.basicAuth({ user: key })
.reply(201, { _id: 1 }, { location: exampleRefLocation });

const spec = './__tests__/__fixtures__/ref-oas/petstore.json';

await expect(
openapi.run({
key,
version,
spec,
})
).resolves.toBe(successfulUpload(spec));

return mock.done();
});

it('should bundle and upload the expected content', async () => {
let requestBody;
Expand Down Expand Up @@ -290,6 +315,41 @@ describe('rdme openapi', () => {

return mock.done();
});

it('should update a spec via prompts', async () => {
prompts.inject(['update', 'spec2']);
const registryUUID = getRandomRegistryId();

const mock = getAPIMock()
.get(`/api/v1/version/${version}`)
.basicAuth({ user: key })
.reply(200, [{ version }])
.post('/api/v1/api-registry', body => body.match('form-data; name="spec"'))
.reply(201, { registryUUID, spec: { openapi: '3.0.0' } })
.get('/api/v1/api-specification')
.basicAuth({ user: key })
.reply(200, [
{ _id: 'spec1', title: 'spec1_title' },
{ _id: 'spec2', title: 'spec2_title' },
])
.put('/api/v1/api-specification/spec2', { registryUUID })
.delayConnection(1000)
.basicAuth({ user: key })
.reply(201, { _id: 1 }, { location: exampleRefLocation });

const spec = './__tests__/__fixtures__/ref-oas/petstore.json';

await expect(
openapi.run({
key,
version,
spec,
})
).resolves.toBe(successfulUpdate(spec));
return mock.done();
});

it.todo('should paginate to next and previous pages of specs');
});

describe('versioning', () => {
Expand Down Expand Up @@ -429,18 +489,15 @@ describe('rdme openapi', () => {
});

it('should request a version list if version is not found', async () => {
promptHandler.generatePrompts.mockResolvedValue({
option: 'create',
newVersion: '1.0.1',
});
prompts.inject(['create', '1.0.1']);

const registryUUID = getRandomRegistryId();

const mock = getAPIMock()
.get('/api/v1/version')
.basicAuth({ user: key })
.reply(200, [{ version: '1.0.0' }])
.post('/api/v1/version')
.post('/api/v1/version', { from: '1.0.0', version: '1.0.1', is_stable: false })
.basicAuth({ user: key })
.reply(200, { from: '1.0.0', version: '1.0.1' })
.post('/api/v1/api-registry', body => body.match('form-data; name="spec"'))
Expand Down
76 changes: 56 additions & 20 deletions __tests__/cmds/versions/create.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import nock from 'nock';
import prompts from 'prompts';

import CreateVersionCommand from '../../../src/cmds/versions/create';
import APIError from '../../../src/lib/apiError';
import getAPIMock from '../../helpers/get-api-mock';

// eslint-disable-next-line @typescript-eslint/no-var-requires
const promptHandler = require('../../../src/lib/prompts');

const key = 'API_KEY';
const version = '1.0.0';

jest.mock('../../../src/lib/prompts');

const createVersion = new CreateVersionCommand();

describe('rdme versions:create', () => {
Expand All @@ -25,32 +21,72 @@ describe('rdme versions:create', () => {
);
});

it('should error if no version provided', () => {
return expect(createVersion.run({ key })).rejects.toStrictEqual(
new Error('Please specify a semantic version. See `rdme help versions:create` for help.')
);
});

it('should error if invaild version provided', () => {
return expect(createVersion.run({ key, version: 'test' })).rejects.toStrictEqual(
new Error('Please specify a semantic version. See `rdme help versions:create` for help.')
);
});

it('should create a specific version', async () => {
promptHandler.createVersionPrompt.mockResolvedValue({
is_stable: true,
is_beta: false,
from: '1.0.0',
});
prompts.inject([version, false, true, true]);
const newVersion = '1.0.1';

const mockRequest = getAPIMock()
.get('/api/v1/version')
.basicAuth({ user: key })
.reply(200, [{ version }, { version }])
.post('/api/v1/version')
.reply(200, [{ version }, { version: '1.1.0' }])
.post('/api/v1/version', {
version: newVersion,
codename: '',
is_stable: false,
is_beta: true,
from: '1.0.0',
is_hidden: false,
})
.basicAuth({ user: key })
.reply(201, { version: newVersion });

await expect(createVersion.run({ key, version: newVersion })).resolves.toBe(
`Version ${newVersion} created successfully.`
);
mockRequest.done();
});

it('should create a specific version with options', async () => {
const newVersion = '1.0.1';

const mockRequest = getAPIMock()
.post('/api/v1/version', {
version: newVersion,
codename: 'test',
from: '1.0.0',
is_hidden: false,
})
.basicAuth({ user: key })
.reply(201, { version });
.reply(201, { version: newVersion });

await expect(
createVersion.run({
key,
version: newVersion,
fork: version,
beta: 'false',
main: 'false',
codename: 'test',
isPublic: 'true',
})
).resolves.toBe(`Version ${newVersion} created successfully.`);

await expect(createVersion.run({ key, version })).resolves.toBe('Version 1.0.0 created successfully.');
mockRequest.done();
});

it('should catch any post request errors', async () => {
expect.assertions(1);
promptHandler.createVersionPrompt.mockResolvedValue({
is_stable: false,
is_beta: false,
});

const errorResponse = {
error: 'VERSION_EMPTY',
message: 'You need to include an x-readme-version header',
Expand Down
Loading

0 comments on commit 8714967

Please sign in to comment.