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

Add tests for helpers readSettings #378

Merged
merged 1 commit into from
Oct 11, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,17 @@ export function readSettings<T>(serviceName: string, settingsFileName: string):
throw new Error(`[${serviceName}] Unable to load configuration file ${settingsFileName}.`);
}

let parseErrors: JSONC.ParseError[];
const parseErrors: JSONC.ParseError[] = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was needed because of https://github.com/danecreekphotography/node-deepstackai-trigger/pull/378/files#r502622394

I wanted to leave this alone, as I'm not sure what will happen at run time of the actual application.


const settings = JSONC.parse(rawConfig, parseErrors) as T;

// This extra level of validation really shouldn't be necessary since the
// file passed schema validation. Still, better safe than crashing.
if (parseErrors && parseErrors.length > 0) {
throw new Error(
`[${serviceName}] Unable to load configuration file: ${parseErrors
.map(error => log.error("${serviceName}", `${error?.error}`))
.join("\n")}`,
);
const parseErrorsAsString = parseErrors.map(parseError => JSON.stringify(parseError)).join(" ");
log.error(serviceName, parseErrorsAsString);
const errorMessage = `[${serviceName}] Unable to load configuration file: ${parseErrorsAsString}`;
throw new Error(errorMessage);
neilenns marked this conversation as resolved.
Show resolved Hide resolved
}

return settings;
Expand Down
116 changes: 116 additions & 0 deletions tests/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Neil Enns. All rights reserved.
* Licensed under the MIT License. See LICENSE in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import {closeSync, existsSync, openSync, unlinkSync, writeFileSync} from 'fs';
import * as JSONC from "jsonc-parser";
import * as helpers from "./../src/helpers";

describe("helpers", () => {
const settingsFilePath = `${__dirname}/settings.json`;

beforeEach(() => {
closeSync(openSync(settingsFilePath, 'w'));
});

afterEach(() => {
jest.clearAllMocks();
if (existsSync(settingsFilePath)) {
unlinkSync(settingsFilePath);
}
});

test("Verify can load settings.json", () => {
const serviceName = "Settings";
const expectedSettings = {"foo": "bar"};
writeFileSync(settingsFilePath, JSON.stringify(expectedSettings));

const actualSettings = helpers.readSettings(serviceName, settingsFilePath);

expect(actualSettings).toEqual(expectedSettings);
});

test("Verify cannot load settings.json because it does not exist", () => {
//eslint-disable-next-line no-console
neilenns marked this conversation as resolved.
Show resolved Hide resolved
console.log = jest.fn();
const serviceName = "Settings";
unlinkSync(settingsFilePath);

const actualSettings = helpers.readSettings(serviceName, settingsFilePath);

//eslint-disable-next-line no-console
expect(console.log).toHaveBeenCalledWith(expect.stringContaining("[Settings] Unable to read the configuration file: ENOENT: no such file or directory"));
expect(actualSettings).toBeNull();
});

test("Verify throws if rawConfig empty", () => {
const serviceName = "Settings";
const expectedSettings = "";
writeFileSync(settingsFilePath, expectedSettings);

expect(() => {helpers.readSettings(serviceName, settingsFilePath)}).toThrow(Error);
});

test("Verify throws with message if rawConfig empty", () => {
const serviceName = "Settings";
const expectedSettings = "";
writeFileSync(settingsFilePath, expectedSettings);

try {
helpers.readSettings(serviceName, settingsFilePath);
} catch (error) {
expect(error.message).toBe(`[${serviceName}] Unable to load configuration file ${settingsFilePath}.`);
}
});
Comment on lines +46 to +64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify a bit here. I write two tests, where one makes for sure that the function threw and the other tests the error message. If I deleted the first of the two and left the second, it's possible that the function does not throw and it goes unnoticed because it wouldn't go into the catch and no assertions is considered a successful test, making it a false-positive.


test("Verify throws if json invalid", () => {
const serviceName = "Settings";
const expectedSettings = {};
writeFileSync(settingsFilePath, JSON.stringify(expectedSettings));
const mockAddListener = jest.spyOn(JSONC, 'parse');
mockAddListener.mockImplementation((rawConfig, parseErrors) => {
parseErrors.push({
error: 1,
offset: 2,
length: 3,
});
Comment on lines +72 to +76
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried:

parseErrors = [{
  error: 1,
  offset: 2,
  length: 3,
}]

but it seems like parseErrors is readonly or something... as I would console.log(parseErrors) and it would output undefined.

return {};
});

expect(() => {helpers.readSettings(serviceName, settingsFilePath)}).toThrow(Error);
});

test("Verify throws with message if json invalid", () => {
const serviceName = "Settings";
const expectedSettings = {};
writeFileSync(settingsFilePath, JSON.stringify(expectedSettings));
const mockAddListener = jest.spyOn(JSONC, 'parse');
const parseError1 = {
error: 1,
offset: 2,
length: 3,
};
const parseError2 = {
error: 3,
offset: 2,
length: 1,
};
mockAddListener.mockImplementation((rawConfig, parseErrors) => {
parseErrors.push(parseError1);
parseErrors.push(parseError2);
return {};
});
neilenns marked this conversation as resolved.
Show resolved Hide resolved
//eslint-disable-next-line no-console
console.log = jest.fn();

try {
helpers.readSettings(serviceName, settingsFilePath);
} catch (error) {
const parseErrorsAsString = `${JSON.stringify(parseError1)} ${JSON.stringify(parseError2)}`;
//eslint-disable-next-line no-console
expect(console.log).toHaveBeenCalledWith(expect.stringContaining(`[${serviceName}] ${parseErrorsAsString}`));
expect(error.message).toBe(`[${serviceName}] Unable to load configuration file: ${parseErrorsAsString}`);
}
});
});