Skip to content

Commit

Permalink
fix(docs): directory sync fix + test bed rewrites (#421)
Browse files Browse the repository at this point in the history 
* chore: rename test name, rename console.log to console.info

I don't like the word 'only' in tests lol, makes searching for .only() tests kinda annoying!

Also using console.info so our console mocking doesn't interfere with our local debugging. ESLint config on this to come!

* fix: make prompt for creating new version more generic

* test(edit): refactor tests to check for resolved/rejected promise values

* docs(edit): add some comments on anti-patterns stuff we're doing

* fix: reject promise if any files fail to upload

* fix: don't allow new version creation in docs syncing

* test: rewrite test for handling invalid docs syncing

* docs: more notes re: categories

* test: remove before/afterEach hooks

We're only using this mock in a single place!

* chore: PR feedback

Co-Authored-By: Jon Ursenbach <jon@ursenba.ch>

Co-authored-by: Jon Ursenbach <jon@ursenba.ch>
  • Loading branch information
@kanadgupta @erunion
kanadgupta and erunion authored Jan 4, 2022
1 parent 0c579b7 commit b793afc
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 106 deletions.
164 changes: 74 additions & 90 deletions __tests__/cmds/docs.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
const nock = require('nock');
const chalk = require('chalk');
const config = require('config');
const fs = require('fs');
const path = require('path');
const crypto = require('crypto');
const frontMatter = require('gray-matter');

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

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

Expand Down Expand Up @@ -186,18 +189,21 @@ describe('rdme docs', () => {
});
});

it('should create only valid docs', () => {
console.log = jest.fn();
expect.assertions(2);

it('should fail if any docs are invalid', async () => {
const folder = 'failure-docs';
const slug = 'fail-doc';
const slugTwo = 'new-doc';

const doc = frontMatter(fs.readFileSync(path.join(fixturesDir, `/failure-docs/${slug}.md`)));
const docTwo = frontMatter(fs.readFileSync(path.join(fixturesDir, `/failure-docs/${slugTwo}.md`)));
const errorObject = {
error: 'DOC_INVALID',
message: "We couldn't save this doc (Path `category` is required.).",
};

const doc = frontMatter(fs.readFileSync(path.join(fixturesDir, `/${folder}/${slug}.md`)));
const docTwo = frontMatter(fs.readFileSync(path.join(fixturesDir, `/${folder}/${slugTwo}.md`)));

const hash = hashFileContents(fs.readFileSync(path.join(fixturesDir, `/failure-docs/${slug}.md`)));
const hashTwo = hashFileContents(fs.readFileSync(path.join(fixturesDir, `/failure-docs/${slugTwo}.md`)));
const hash = hashFileContents(fs.readFileSync(path.join(fixturesDir, `/${folder}/${slug}.md`)));
const hashTwo = hashFileContents(fs.readFileSync(path.join(fixturesDir, `/${folder}/${slugTwo}.md`)));

const getMocks = getNockWithVersionHeader(version)
.get(`/api/v1/docs/${slug}`)
Expand Down Expand Up @@ -238,43 +244,27 @@ describe('rdme docs', () => {
})
.post('/api/v1/docs', { slug, body: doc.content, ...doc.data, lastUpdatedHash: hash })
.basicAuth({ user: key })
.reply(400, {
error: 'DOC_INVALID',
message: "We couldn't save this doc (Path `category` is required.).",
});
.reply(400, errorObject);

const versionMock = nock(config.host)
.get(`/api/v1/version/${version}`)
.basicAuth({ user: key })
.reply(200, { version });

return docs.run({ folder: './__tests__/__fixtures__/failure-docs', key, version }).then(message => {
expect(console.log).toHaveBeenCalledTimes(1);
expect(message).toStrictEqual([
{
metadata: { image: [], title: '', description: '' },
api: {
method: 'post',
url: '',
auth: 'required',
params: [],
apiSetting,
},
title: 'This is the document title',
updates: [],
type: 'endpoint',
slug: slugTwo,
body: 'Body',
category,
},
]);
const fullDirectory = `__tests__/__fixtures__/${folder}`;

getMocks.done();
postMocks.done();
versionMock.done();
const formattedErrorObject = {
...errorObject,
message: `Error uploading ${chalk.underline(`${fullDirectory}/${slug}.md`)}:\n\n${errorObject.message}`,
};

console.log.mockRestore();
});
await expect(docs.run({ folder: `./${fullDirectory}`, key, version })).rejects.toStrictEqual(
new APIError(formattedErrorObject)
);

getMocks.done();
postMocks.done();
versionMock.done();
});
});

Expand Down Expand Up @@ -314,14 +304,6 @@ describe('rdme docs', () => {
});

describe('rdme docs:edit', () => {
beforeEach(() => {
console.log = jest.fn();
});

afterEach(() => {
console.log.mockRestore();
});

it('should error if no api key provided', () => {
return expect(docsEdit.run({})).rejects.toThrow('No project API key provided. Please use `--key`.');
});
Expand All @@ -332,8 +314,9 @@ describe('rdme docs:edit', () => {
);
});

it('should fetch the doc from the api', () => {
expect.assertions(4);
it('should fetch the doc from the api', async () => {
expect.assertions(5);
console.info = jest.fn();
const slug = 'getting-started';
const body = 'abcdef';
const edits = 'ghijkl';
Expand Down Expand Up @@ -363,55 +346,55 @@ describe('rdme docs:edit', () => {
fs.appendFile(filename, edits, cb.bind(null, 0));
}

return docsEdit.run({ slug, key, version: '1.0.0', mockEditor }).then(() => {
getMock.done();
putMock.done();
versionMock.done();
expect(fs.existsSync(`${slug}.md`)).toBe(false);
await expect(docsEdit.run({ slug, key, version: '1.0.0', mockEditor })).resolves.toBeUndefined();

expect(console.log).toHaveBeenCalledWith('Doc successfully updated. Cleaning up local file.');
});
getMock.done();
putMock.done();
versionMock.done();

expect(fs.existsSync(`${slug}.md`)).toBe(false);
expect(console.info).toHaveBeenCalledWith('Doc successfully updated. Cleaning up local file.');
return console.info.mockRestore();
});

it('should error if remote doc does not exist', () => {
expect.assertions(2);
it('should error if remote doc does not exist', async () => {
const slug = 'no-such-doc';

const getMock = nock(config.host)
.get(`/api/v1/docs/${slug}`)
.reply(404, {
error: 'DOC_NOTFOUND',
message: `The doc with the slug '${slug}' couldn't be found`,
suggestion: '...a suggestion to resolve the issue...',
help: 'If you need help, email support@readme.io and mention log "fake-metrics-uuid".',
});
const errorObject = {
error: 'DOC_NOTFOUND',
message: `The doc with the slug '${slug}' couldn't be found`,
suggestion: '...a suggestion to resolve the issue...',
help: 'If you need help, email support@readme.io and mention log "fake-metrics-uuid".',
};

const getMock = nock(config.host).get(`/api/v1/docs/${slug}`).reply(404, errorObject);

const versionMock = nock(config.host)
.get(`/api/v1/version/${version}`)
.basicAuth({ user: key })
.reply(200, { version });

return docsEdit.run({ slug, key, version: '1.0.0' }).catch(err => {
getMock.done();
versionMock.done();
expect(err.code).toBe('DOC_NOTFOUND');
expect(err.message).toContain("The doc with the slug 'no-such-doc' couldn't be found");
});
await expect(docsEdit.run({ slug, key, version: '1.0.0' })).rejects.toThrow(new APIError(errorObject));

getMock.done();
return versionMock.done();
});

it('should error if doc fails validation', () => {
it('should error if doc fails validation', async () => {
expect.assertions(2);
const slug = 'getting-started';
const body = 'abcdef';

const getMock = nock(config.host).get(`/api/v1/docs/${slug}`).reply(200, { body });

const putMock = nock(config.host).put(`/api/v1/docs/${slug}`).reply(400, {
const errorObject = {
error: 'DOC_INVALID',
message: "We couldn't save this doc ({error})",
message: `We couldn't save this doc (${slug})`,
suggestion: '...a suggestion to resolve the issue...',
help: 'If you need help, email support@readme.io and mention log "fake-metrics-uuid".',
});
};

const getMock = nock(config.host).get(`/api/v1/docs/${slug}`).reply(200, { body });

const putMock = nock(config.host).put(`/api/v1/docs/${slug}`).reply(400, errorObject);

const versionMock = nock(config.host)
.get(`/api/v1/version/${version}`)
Expand All @@ -422,17 +405,17 @@ describe('rdme docs:edit', () => {
return cb(0);
}

return docsEdit.run({ slug, key, version: '1.0.0', mockEditor }).catch(err => {
expect(err.code).toBe('DOC_INVALID');
getMock.done();
putMock.done();
versionMock.done();
expect(fs.existsSync(`${slug}.md`)).toBe(true);
fs.unlinkSync(`${slug}.md`);
});
await expect(docsEdit.run({ slug, key, version: '1.0.0', mockEditor })).rejects.toThrow(new APIError(errorObject));

getMock.done();
putMock.done();
versionMock.done();

expect(fs.existsSync(`${slug}.md`)).toBe(true);
fs.unlinkSync(`${slug}.md`);
});

it('should handle error if $EDITOR fails', () => {
it('should handle error if $EDITOR fails', async () => {
expect.assertions(1);
const slug = 'getting-started';
const body = 'abcdef';
Expand All @@ -448,10 +431,11 @@ describe('rdme docs:edit', () => {
return cb(1);
}

return docsEdit.run({ slug, key, version: '1.0.0', mockEditor }).catch(err => {
getMock.done();
expect(err.message).toBe('Non zero exit code from $EDITOR');
fs.unlinkSync(`${slug}.md`);
});
await expect(docsEdit.run({ slug, key, version: '1.0.0', mockEditor })).rejects.toThrow(
new Error('Non zero exit code from $EDITOR')
);

getMock.done();
fs.unlinkSync(`${slug}.md`);
});
});
8 changes: 7 additions & 1 deletion src/cmds/docs/edit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,17 @@ exports.run = async function (opts) {
})
.then(res => res.json())
.then(async res => {
// The reason we aren't using our handleRes() function here is
// because we need to use the `reject` function from
// the Promise that's wrapping this function.
if (res.error) {
return reject(new APIError(res));
}
console.log(`Doc successfully updated. Cleaning up local file.`);
console.info(`Doc successfully updated. Cleaning up local file.`);
await unlink(filename);
// Normally we should resolve with a value that is logged to the console,
// but since we need to wait for the temporary file to be removed,
// it's okay to resolve the promise with no value.
return resolve();
});
});
Expand Down
22 changes: 8 additions & 14 deletions src/cmds/docs/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,10 @@ exports.run = async function (opts) {
return Promise.reject(new Error(`No folder provided. Usage \`${config.cli} ${exports.usage}\`.`));
}

const selectedVersion = await getProjectVersion(version, key, true).catch(e => {
return Promise.reject(e);
});
// TODO: should we allow version selection at all here?
// Let's revisit this once we re-evaluate our category logic in the API.
// Ideally we should ignore this parameter entirely if the category is included.
const selectedVersion = await getProjectVersion(version, key, false);

// Find the files to sync
const readdirRecursive = folderToSearch => {
Expand Down Expand Up @@ -110,7 +111,7 @@ exports.run = async function (opts) {
}).then(res => handleRes(res));
}

const updatedDocs = await Promise.allSettled(
const updatedDocs = await Promise.all(
files.map(async filename => {
const file = await readFile(filename, 'utf8');
const matter = frontMatter(file);
Expand All @@ -134,19 +135,12 @@ exports.run = async function (opts) {
return updateDoc(slug, matter, hash, res);
})
.catch(err => {
console.log(chalk.red(`\n\`${slug}\` failed to upload. ${err.message}\n`));
// eslint-disable-next-line no-param-reassign
err.message = `Error uploading ${chalk.underline(filename)}:\n\n${err.message}`;
throw err;
});
})
);

for (let i = 0; i < updatedDocs.length; ) {
if (updatedDocs[i].value !== undefined) {
updatedDocs[i] = updatedDocs[i].value; // returns only the value of the response
i += 1;
} else {
updatedDocs.splice(i, 1); // we already displayed the error messages so we can filter those out
}
}

return updatedDocs;
};
2 changes: 1 addition & 1 deletion src/lib/prompts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ exports.generatePrompts = (versionList, selectOnly = false) => [
{
type: 'select',
name: 'option',
message: 'Would you like to use an existing version or create a new one to associate with your OAS file?',
message: 'Would you like to use an existing project version or create a new one?',
skip() {
return selectOnly;
},
Expand Down

0 comments on commit b793afc

Please sign in to comment.