Skip to content

Commit

Permalink
Implement KindError (#16)
Browse files Browse the repository at this point in the history
## Summary:
This implements `KindError`, an error type to act as the basis for our various `KAError` versions.
This includes code to combine messages and stack traces in a meaningful way between a source error and the error being constructed around that.

Also:

 - Enable no-default-exports lint rule
   - Named exports provide an explicit API that is easy to search, rename, etc.
   - Required updating to @babel/eslint-parser
 - Excludes index.js and types.js from coverage data
 - Adds jest-when

This doesn't add any of the useful helper methods from mobile yet or anything sentry specific.
The sentry stuff will be in a different package than core and the helpers will be added in an upcoming PR (either to KindError or a more specific derivation).
Some of the helpers should be unnecessary now as the constructor uses an `options` arg to simplify the provision of various options.

Issue: FEI-3997

## Test plan:
`yarn test`
`yarn flow`
`yarn lint`
`yarn build`

I also want to create some sort of E2E/integration test (see FEI-4058 for details on that)

Author: somewhatabstract

Reviewers: somewhatabstract, lillialexis

Required Reviewers:

Approved By: lillialexis

Checks: ✅ codecov/project, ✅ CodeQL, ✅ Test (macOS-latest, 14.x), ✅ Test (macOS-latest, 12.x), ✅ Test (ubuntu-latest, 12.x), ✅ Gather coverage (ubuntu-latest, 14.x), ✅ Lint and flow check (ubuntu-latest, 14.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 14.x), ✅ Analyze (javascript), ⏭  dependabot

Pull Request URL: #16
  • Loading branch information
somewhatabstract authored Oct 14, 2021
1 parent 22b238d commit fbc363d
Show file tree
Hide file tree
Showing 19 changed files with 1,238 additions and 79 deletions.
18 changes: 17 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
/* eslint-disable import/no-commonjs */
module.exports = {
extends: ["@khanacademy"],
plugins: ["import", "jest", "promise", "monorepo", "disable"],
parser: "@babel/eslint-parser",
parserOptions: {
sourceType: "module",
babelOptions: {
configFile: "./build-settings/babel.config.js",
},
},
plugins: ["@babel", "import", "jest", "promise", "monorepo", "disable"],
settings: {
"eslint-plugin-disable": {
paths: {
Expand All @@ -25,8 +32,17 @@ module.exports = {
},
],
rules: {
"new-cap": "off",
"no-invalid-this": "off",
"object-curly-spacing": "off",
semi: "off",
"@babel/new-cap": "error",
"@babel/no-invalid-this": "error",
"@babel/object-curly-spacing": "error",
"@babel/semi": "error",
"flowtype/require-exact-type": ["error", "always"],
"flowtype/no-types-missing-file-annotation": "error",
"import/no-default-export": "error",
"import/no-unresolved": "error",
"import/named": "error",
"import/default": "error",
Expand Down
5 changes: 5 additions & 0 deletions .quokka
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"babel": {
"configFile": "./build-settings/babel.config.js"
}
}
7 changes: 6 additions & 1 deletion config/jest/test.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,18 @@ module.exports = {
},
testEnvironment: "jest-environment-node",
testMatch: ["<rootDir>/**/*.test.js"],
setupFilesAfterEnv: ["<rootDir>/config/jest/test-setup.js"],
setupFilesAfterEnv: [
"jest-extended/all",
"<rootDir>/config/jest/test-setup.js",
],
moduleNameMapper: {
"^@khanacademy/wonder-stuff-(.*)$":
"<rootDir>/packages/wonder-stuff-$1/src/index.js",
},
collectCoverageFrom: [
"packages/**/*.js",
"!packages/**/types.js",
"!packages/**/src/index.js",
"!packages/**/*.flowtest.js",
"!packages/**/dist/**/*.js",
"!<rootDir>/node_modules/",
Expand Down
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
],
"devDependencies": {
"@babel/core": "^7.15.8",
"@babel/eslint-parser": "^7.15.8",
"@babel/eslint-plugin": "^7.14.5",
"@babel/preset-env": "^7.15.8",
"@babel/preset-flow": "^7.14.5",
"@khanacademy/eslint-config": "^0.1.0",
"@rollup/plugin-babel": "^5.3.0",
"@rollup/plugin-node-resolve": "^13.0.5",
"@rollup/plugin-replace": "^3.0.0",
"babel-eslint": "^10.1.0",
"babel-jest": "^27.2.4",
"eslint": "^7.32.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-babel": "^5.3.1",
Expand All @@ -37,6 +38,7 @@
"flow-bin": "^0.161.0",
"jest": "^27.2.4",
"jest-extended": "^1.0.0",
"jest-when": "^3.4.1",
"lerna": "^4.0.0",
"prettier": "^2.4.1",
"rollup": "^2.57.0",
Expand All @@ -55,5 +57,6 @@
"publishchecks": "git diff --stat --exit-code HEAD && yarn build && node utils/pre-publish-check.js",
"publish": "yarn run publishchecks && lerna publish",
"test": "yarn jest"
}
},
"dependencies": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`#buildCausedByMessage should default nullish and whitespace strings to (empty message) 1`] = `
"(empty message)
caused by
(empty message)"
`;

exports[`#buildCausedByMessage should default nullish and whitespace strings to (empty message) 2`] = `
"(empty message)
caused by
(empty message)"
`;

exports[`#buildCausedByMessage should default nullish and whitespace strings to (empty message) 3`] = `
"(empty message)
caused by
(empty message)"
`;

exports[`#buildCausedByMessage should default nullish and whitespace strings to (empty message) 4`] = `
"(empty message)
caused by
(empty message)"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`KindError #constructor should throw if the kind has whitespace like K I N D 1`] = `"kind must not contain whitespace"`;

exports[`KindError #constructor should throw if the kind has whitespace like KI
ND 1`] = `"kind must not contain whitespace"`;

exports[`KindError #constructor should throw if the prefix has whitespace like P R E F I X 1`] = `"prefix must not contain whitespace"`;

exports[`KindError #constructor should throw if the prefix has whitespace like PRE
FIX 1`] = `"prefix must not contain whitespace"`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// @flow
import {buildCausedByMessage} from "../build-caused-by-message.js";

describe("#buildCausedByMessage", () => {
it("should combine the strings into a 'caused by' message", () => {
// Arrange
const consequence = "No more Halloween candy";
const cause = "Gluttonous parents";

// Act
const result = buildCausedByMessage(consequence, cause);

// Assert
expect(result).toMatchInlineSnapshot(`
"No more Halloween candy
caused by
Gluttonous parents"
`);
});

it.each([null, undefined, " ", "\n\t\t"])(
"should default nullish and whitespace strings to (empty message)",
(testCase) => {
// Arrange
const consequence = testCase;
const cause = testCase;

// Act
const result = buildCausedByMessage(consequence, cause);

// Assert
expect(result).toMatchSnapshot();
},
);
});
141 changes: 141 additions & 0 deletions packages/wonder-stuff-core/src/__tests__/clone-metadata.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// @flow
import {cloneMetadata} from "../clone-metadata.js";

describe("#cloneMetadata", () => {
it.each([undefined, null])(
"should return the metadata when it is %s",
(value) => {
// Arrange

// Act
const result = cloneMetadata(value);

// Assert
expect(result).toBe(value);
},
);

it("should freeze the clone", () => {
// Arrange
const spy = jest.spyOn(Object, "freeze");

// Act
const result = cloneMetadata({});

// Assert
expect(spy).toHaveBeenCalledWith(result);
});

it("should clone a simple object", () => {
// Arrange
const metadata = {
string: "bar",
number: "42",
boolean: "true",
undefined: undefined,
null: null,
};

// Act
const result = cloneMetadata(metadata);
metadata.string = "baz";
metadata.number = "43";
metadata.boolean = "false";
metadata.undefined = "foo";
// $FlowIgnore[incompatible-type]
delete metadata.null;

// Assert
expect(result).toEqual({
string: "bar",
number: "42",
boolean: "true",
undefined: undefined,
null: null,
});
expect(result).not.toEqual(metadata);
});

it("should clone arrays", () => {
// Arrange
const metadata = {
array: [1, 2, 3],
};

// Act
const result = cloneMetadata(metadata);
metadata.array.push(4);

// Assert
expect(result).toEqual({
array: [1, 2, 3],
});
expect(result).not.toEqual(metadata);
});

it("should clone objects", () => {
// Arrange
const metadata = {
object: {
a: 1,
b: 2,
},
};

// Act
const result = cloneMetadata(metadata);
metadata.object.a = 3;

// Assert
expect(result).toEqual({
object: {
a: 1,
b: 2,
},
});
expect(result).not.toEqual(metadata);
});

it("should clone nested objects and arrays", () => {
// Arrange
const metadata = {
object: {
a: 1,
b: 2,
c: {
d: 3,
e: [
4,
5,
{
nested: "in here",
},
],
},
},
};

// Act
const result = cloneMetadata(metadata);
metadata.object.c.e[2].nested = "in there";

// Assert
expect(result).toEqual({
object: {
a: 1,
b: 2,
c: {
d: 3,
e: [
4,
5,
{
nested: "in here",
},
],
},
},
});
expect(result).not.toEqual(metadata);
});
});
Loading

0 comments on commit fbc363d

Please sign in to comment.