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.

Sign up for GitHub

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

Jump to bottom

Refactor test helpers #1336

Merged
merged 25 commits into from Jul 22, 2018
Merged

Refactor test helpers #1336

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b3dcf40
Refactor test helpers
ylemkimon May 21, 2018
6cdff3c
Update helpers.js
ylemkimon May 21, 2018
d751297
Merge branch 'master' into jest-test-helpers
ylemkimon May 21, 2018
4250d06
Remove toBeTruthy checks
ylemkimon May 24, 2018
7343e48
Merge branch 'master' into jest-test-helpers
ylemkimon Jun 7, 2018
0c96b20
Merge branch 'master' into jest-test-helpers
ylemkimon Jun 16, 2018
6e1773b
Merge branch 'jest-test-helpers' of https://github.com/ylemkimon/KaTe…
ylemkimon Jun 17, 2018
17ca384
Used tagged literals
ylemkimon Jun 17, 2018
721f62f
Use .toHaveLength
ylemkimon Jun 17, 2018
a8c91eb
Use tagged literals
ylemkimon Jun 17, 2018
96cdfa0
Use to{Build,Parse}Like where possible
ylemkimon Jun 17, 2018
4ad8dfd
Use snapshot where possible
ylemkimon Jun 17, 2018
aeb5350
Join into one line where <88 chars
ylemkimon Jun 17, 2018
4e84e93
Revert console.warn() to throw an error
ylemkimon Jun 18, 2018
4e387b8
Remove call to console.warn from stack traces
ylemkimon Jun 18, 2018
6f6865d
Merge branch 'master' into jest-test-helpers
ylemkimon Jun 18, 2018
d89600c
Merge branch 'master' into jest-test-helpers
ylemkimon Jun 24, 2018
527c5b3
Merge branch 'master' into jest-test-helpers
ylemkimon Jun 27, 2018
92f13b1
Merge branch 'master' into jest-test-helpers
ylemkimon Jul 1, 2018
a49c067
Fix merge errors
ylemkimon Jul 8, 2018
6572122
Merge branch 'master' into jest-test-helpers
ylemkimon Jul 8, 2018
f783eb8
Remove `getTree`
ylemkimon Jul 8, 2018
fc47230
Extract `expected` string construction into `printExpectedResult`
ylemkimon Jul 9, 2018
b557735
Merge branch 'master' into jest-test-helpers
ylemkimon Jul 16, 2018
c64ef13
Merge branch 'master' into jest-test-helpers
kevinbarabash Jul 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 0 additions & 20 deletions test/Warning.js
View file
Open in desktop

This file was deleted.

22 changes: 22 additions & 0 deletions test/__snapshots__/katex-spec.js.snap
View file
Open in desktop
Expand Up @@ -585,6 +585,28 @@ exports[`A font parser \\boldsymbol should inherit mbin/mrel from argument 1`] =
]
`;

exports[`A parse tree generator generates a tree 1`] = `
[
{
"type": "supsub",
"mode": "math",
"value": {
"type": "supsub",
"base": {
"type": "mathord",
"mode": "math",
"value": "\\\\sigma"
},
"sup": {
"type": "textord",
"mode": "math",
"value": "2"
}
}
}
]
`;

exports[`A parser that does not throw on unsupported commands should build katex-error span for other type of KaTeX error 1`] = `
{
"attributes": {
Expand Down
100 changes: 50 additions & 50 deletions test/errors-spec.js
View file
Open in desktop
Expand Up @@ -6,76 +6,76 @@ describe("Parser:", function() {

describe("#handleInfixNodes", function() {
it("rejects repeated infix operators", function() {
expect("1\\over 2\\over 3").toFailWithParseError(
expect`1\over 2\over 3`.toFailWithParseError(
Copy link
Member

Choose a reason for hiding this comment

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

This looks a little strange to me b/c I'm not used to template strings returning things that aren't strings. Maybe we could introduce a tex template literal function. It would look like:

expect(tex`1\over 2\over 3`).toFailWithParseError(...

The tex function could be exported for use by other people. Also, it would be easier to understand what's going on. I'm still not sure how expect is acting as a template function without us modifying/replacing it with our own implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kevinbarabash I agree that it can be unfamiliar since it's new syntax. However, it's new way to call the function with arguments: an array of strings with raw property, an array of raw strings, and expressions in ${}. As expect accepts any object, it just passes the first argument through and we can handle them in the matchers. Maybe we can document them in the CONTRIBUTING.md?

Copy link
Member

@edemaine edemaine Jul 8, 2018 •
edited

Choose a reason for hiding this comment

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

If expect passes through only one argument, does that mean this won't work with ${interpolated expressions}?

I don't think the new code looks much more confusing than the old code -- the weirdest part is that expect just passes on its argument, and the action is in the chained methods afterward -- but that's a general Jest thing. However, until I read the template literal description I didn't really understand what the code was doing.

What about expectTex`1 \over 2 \over 3` which is equivalent to expect(tex`1 \over 2 \over 3`), i.e., the string gets processed before calling expect, instead of requiring all matches to process it? I think the weirdest part is overloading expect in this unintended way (unless Jest suggests doing this?).

Copy link
Member Author

@ylemkimon ylemkimon Jul 8, 2018 •
edited

Choose a reason for hiding this comment

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

@edemaine Yes, it will throw an error if there are ${interpolated expressions} and r will return the part before the first one.

I think we can think of expect.toParse as expect the raw string of the tagged literal to success parsing.

Copy link
Member Author

@ylemkimon ylemkimon Jul 8, 2018 •
edited

Choose a reason for hiding this comment

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

It's not related, but Jest provides a way of creating a table using tagged template literal:

describe.each`
  a    | b    | expected
  ${1} | ${1} | ${2}
  ${1} | ${2} | ${3}
  ${2} | ${1} | ${3}
`('$a + $b', ({a, b, expected}) => {
  test(`returns ${expected}`, () => {
    expect(a + b).toBe(expected);
  });

  test(`returned value not be greater than ${expected}`, () => {
    expect(a + b).not.toBeGreaterThan(expected);
  });

  test(`returned value not be less than ${expected}`, () => {
    expect(a + b).not.toBeLessThan(expected);
  });
});

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. I was unaware of jest's use of tagged template literals. expectTex seems like a good compromise.

Copy link
Member

Choose a reason for hiding this comment

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

@ylemkimon apologies for not raising my concerns about expect as a tagged template earlier.

Copy link
Member Author

@ylemkimon ylemkimon Jul 9, 2018 •
edited

Choose a reason for hiding this comment

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

@kevinbarabash @edemaine

@ylemkimon apologies for not raising my concerns about expect as a tagged template earlier.

I don't mind. I think productive discussion is necessary.

What about expectTex`1 \over 2 \over 3` which is equivalent to expect(tex`1 \over 2 \over 3`), i.e., the string gets processed before calling expect, instead of requiring all matches to process it? I think the weirdest part is overloading expect in this unintended way (unless Jest suggests doing this?).

Interesting. I was unaware of jest's use of tagged template literals. expectTex seems like a good compromise.

I still disagree. The purpose of using tagged template literal is to avoid using escape backslashes and simplify test cases. I think using expectTeX defeats this purpose as there is no difference between processing before passing to expect and processing in the matchers, when using KaTeX matchers.

Copy link
Member

Choose a reason for hiding this comment

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

@ylemkimon There would be a difference if we needed to use Jest's matchers. But I can't actually think of any situation where we'd want to use those on a TeX string.

"only one infix operator per group at position 9: " +
"1\\over 2\\̲o̲v̲e̲r̲ ̲3");
});
it("rejects conflicting infix operators", function() {
expect("1\\over 2\\choose 3").toFailWithParseError(
expect`1\over 2\choose 3`.toFailWithParseError(
"only one infix operator per group at position 9: " +
"1\\over 2\\̲c̲h̲o̲o̲s̲e̲ ̲3");
});
});

describe("#handleSupSubscript", function() {
it("rejects ^ at end of group", function() {
expect("{1^}").toFailWithParseError(
expect`{1^}`.toFailWithParseError(
"Expected group after '^' at position 3: {1^̲}");
});
it("rejects _ at end of input", function() {
expect("1_").toFailWithParseError(
expect`1_`.toFailWithParseError(
"Expected group after '_' at position 2: 1_̲");
});
it("rejects \\sqrt as argument to ^", function() {
expect("1^\\sqrt{2}").toFailWithParseError(
expect`1^\sqrt{2}`.toFailWithParseError(
"Got function '\\sqrt' with no arguments as superscript" +
" at position 2: 1^̲\\sqrt{2}");
});
});

describe("#parseAtom", function() {
it("rejects \\limits without operator", function() {
expect("\\alpha\\limits\\omega").toFailWithParseError(
expect`\alpha\limits\omega`.toFailWithParseError(
"Limit controls must follow a math operator" +
" at position 7: \\alpha\\̲l̲i̲m̲i̲t̲s̲\\omega");
});
it("rejects \\limits at the beginning of the input", function() {
expect("\\limits\\omega").toFailWithParseError(
expect`\limits\omega`.toFailWithParseError(
"Limit controls must follow a math operator" +
" at position 1: \\̲l̲i̲m̲i̲t̲s̲\\omega");
});
it("rejects double superscripts", function() {
expect("1^2^3").toFailWithParseError(
expect`1^2^3`.toFailWithParseError(
"Double superscript at position 4: 1^2^̲3");
expect("1^{2+3}_4^5").toFailWithParseError(
expect`1^{2+3}_4^5`.toFailWithParseError(
"Double superscript at position 10: 1^{2+3}_4^̲5");
});
it("rejects double superscripts involving primes", function() {
expect("1'_2^3").toFailWithParseError(
expect`1'_2^3`.toFailWithParseError(
"Double superscript at position 5: 1'_2^̲3");
expect("1^2'").toFailWithParseError(
expect`1^2'`.toFailWithParseError(
"Double superscript at position 4: 1^2'̲");
expect("1^2_3'").toFailWithParseError(
expect`1^2_3'`.toFailWithParseError(
"Double superscript at position 6: 1^2_3'̲");
expect("1'_2'").toFailWithParseError(
expect`1'_2'`.toFailWithParseError(
"Double superscript at position 5: 1'_2'̲");
});
it("rejects double subscripts", function() {
expect("1_2_3").toFailWithParseError(
expect`1_2_3`.toFailWithParseError(
"Double subscript at position 4: 1_2_̲3");
expect("1_{2+3}^4_5").toFailWithParseError(
expect`1_{2+3}^4_5`.toFailWithParseError(
"Double subscript at position 10: 1_{2+3}^4_̲5");
});
});

describe("#parseImplicitGroup", function() {
it("reports unknown environments", function() {
expect("\\begin{foo}bar\\end{foo}").toFailWithParseError(
expect`\begin{foo}bar\end{foo}`.toFailWithParseError(
"No such environment: foo at position 7:" +
" \\begin{̲f̲o̲o̲}̲bar\\end{foo}");
});
it("reports mismatched environments", function() {
expect("\\begin{pmatrix}1&2\\\\3&4\\end{bmatrix}+5")
expect`\begin{pmatrix}1&2\\3&4\end{bmatrix}+5`
.toFailWithParseError(
"Mismatch: \\begin{pmatrix} matched by \\end{bmatrix}" +
" at position 24: …matrix}1&2\\\\3&4\\̲e̲n̲d̲{bmatrix}+5");
Expand All @@ -84,56 +84,56 @@ describe("Parser:", function() {

describe("#parseFunction", function() {
it("rejects math-mode functions in text mode", function() {
expect("\\text{\\sqrt2 is irrational}").toFailWithParseError(
expect`\text{\sqrt2 is irrational}`.toFailWithParseError(
"Can't use function '\\sqrt' in text mode" +
" at position 7: \\text{\\̲s̲q̲r̲t̲2 is irrational…");
});
it("rejects text-mode-only functions in math mode", function() {
expect("\\'echec").toFailWithParseError(
expect`\'echec`.toFailWithParseError(
"Can't use function '\\'' in math mode" +
" at position 1: \\̲'̲echec");
});
});

describe("#parseArguments", function() {
it("complains about missing argument at end of input", function() {
expect("2\\sqrt").toFailWithParseError(
expect`2\sqrt`.toFailWithParseError(
"Expected group after '\\sqrt' at end of input: 2\\sqrt");
});
it("complains about missing argument at end of group", function() {
expect("1^{2\\sqrt}").toFailWithParseError(
expect`1^{2\sqrt}`.toFailWithParseError(
"Expected group after '\\sqrt'" +
" at position 10: 1^{2\\sqrt}̲");
});
it("complains about functions as arguments to others", function() {
// TODO: The position looks pretty wrong here
expect("\\sqrt\\over2").toFailWithParseError(
expect`\sqrt\over2`.toFailWithParseError(
"Got function '\\over' as argument to '\\sqrt'" +
" at position 6: \\sqrt\\̲o̲v̲e̲r̲2");
});
});

describe("#parseArguments", function() {
it("complains about missing argument at end of input", function() {
expect("2\\sqrt").toFailWithParseError(
expect`2\sqrt`.toFailWithParseError(
"Expected group after '\\sqrt' at end of input: 2\\sqrt");
});
it("complains about missing argument at end of group", function() {
expect("1^{2\\sqrt}").toFailWithParseError(
expect`1^{2\sqrt}`.toFailWithParseError(
"Expected group after '\\sqrt'" +
" at position 10: 1^{2\\sqrt}̲");
});
it("complains about functions as arguments to others", function() {
// TODO: The position looks pretty wrong here
expect("\\sqrt\\over2").toFailWithParseError(
expect`\sqrt\over2`.toFailWithParseError(
"Got function '\\over' as argument to '\\sqrt'" +
" at position 6: \\sqrt\\̲o̲v̲e̲r̲2");
});
});

describe("#verb", function() {
it("complains about mismatched \\verb with end of string", function() {
expect("\\verb|hello").toFailWithParseError(
expect`\verb|hello`.toFailWithParseError(
"\\verb ended by end of line instead of matching delimiter");
});
it("complains about mismatched \\verb with end of line", function() {
Expand All @@ -148,28 +148,28 @@ describe("Parser.expect calls:", function() {

describe("#parseInput expecting EOF", function() {
it("complains about extra }", function() {
expect("{1+2}}").toFailWithParseError(
expect`{1+2}}`.toFailWithParseError(
"Expected 'EOF', got '}' at position 6: {1+2}}̲");
});
it("complains about extra \\end", function() {
expect("x\\end{matrix}").toFailWithParseError(
expect`x\end{matrix}`.toFailWithParseError(
"Expected 'EOF', got '\\end' at position 2:" +
" x\\̲e̲n̲d̲{matrix}");
});
it("complains about top-level &", function() {
expect("1&2").toFailWithParseError(
expect`1&2`.toFailWithParseError(
"Expected 'EOF', got '&' at position 2: 1&̲2");
});
});

describe("#parseImplicitGroup expecting \\right", function() {
it("rejects missing \\right", function() {
expect("\\left(1+2)").toFailWithParseError(
expect`\left(1+2)`.toFailWithParseError(
"Expected '\\right', got 'EOF' at end of input:" +
" \\left(1+2)");
});
it("rejects incorrectly scoped \\right", function() {
expect("{\\left(1+2}\\right)").toFailWithParseError(
expect`{\left(1+2}\right)`.toFailWithParseError(
"Expected '\\right', got '}' at position 11:" +
" {\\left(1+2}̲\\right)");
});
Expand All @@ -180,51 +180,51 @@ describe("Parser.expect calls:", function() {

describe("#parseSpecialGroup expecting braces", function() {
it("complains about missing { for color", function() {
expect("\\textcolor#ffffff{text}").toFailWithParseError(
expect`\textcolor#ffffff{text}`.toFailWithParseError(
"Expected '{', got '#' at position 11:" +
" \\textcolor#̲ffffff{text}");
});
it("complains about missing { for size", function() {
expect("\\rule{1em}[2em]").toFailWithParseError(
expect`\rule{1em}[2em]`.toFailWithParseError(
"Invalid size: '[' at position 11: \\rule{1em}[̲2em]");
});
// Can't test for the [ of an optional group since it's optional
it("complains about missing } for color", function() {
expect("\\textcolor{#ffffff{text}").toFailWithParseError(
expect`\textcolor{#ffffff{text}`.toFailWithParseError(
"Invalid color: '#ffffff{text' at position 12:" +
" \\textcolor{#̲f̲f̲f̲f̲f̲f̲{̲t̲e̲x̲t̲}");
});
it("complains about missing ] for size", function() {
expect("\\rule[1em{2em}{3em}").toFailWithParseError(
expect`\rule[1em{2em}{3em}`.toFailWithParseError(
"Unexpected end of input in size" +
" at position 7: \\rule[1̲e̲m̲{̲2̲e̲m̲}̲{̲3̲e̲m̲}̲");
});
it("complains about missing ] for size at end of input", function() {
expect("\\rule[1em").toFailWithParseError(
expect`\rule[1em`.toFailWithParseError(
"Unexpected end of input in size" +
" at position 7: \\rule[1̲e̲m̲");
});
it("complains about missing } for color at end of input", function() {
expect("\\textcolor{#123456").toFailWithParseError(
expect`\textcolor{#123456`.toFailWithParseError(
"Unexpected end of input in color" +
" at position 12: \\textcolor{#̲1̲2̲3̲4̲5̲6̲");
});
});

describe("#parseGroup expecting }", function() {
it("at end of file", function() {
expect("\\sqrt{2").toFailWithParseError(
expect`\sqrt{2`.toFailWithParseError(
"Expected '}', got 'EOF' at end of input: \\sqrt{2");
});
});

describe("#parseOptionalGroup expecting ]", function() {
it("at end of file", function() {
expect("\\sqrt[3").toFailWithParseError(
expect`\sqrt[3`.toFailWithParseError(
"Expected ']', got 'EOF' at end of input: \\sqrt[3");
});
it("before group", function() {
expect("\\sqrt[3{2}").toFailWithParseError(
expect`\sqrt[3{2}`.toFailWithParseError(
"Expected ']', got 'EOF' at end of input: \\sqrt[3{2}");
});
});
Expand All @@ -235,12 +235,12 @@ describe("environments.js:", function() {

describe("parseArray", function() {
it("rejects missing \\end", function() {
expect("\\begin{matrix}1").toFailWithParseError(
expect`\begin{matrix}1`.toFailWithParseError(
"Expected & or \\\\ or \\cr or \\end at end of input:" +
" \\begin{matrix}1");
});
it("rejects incorrectly scoped \\end", function() {
expect("{\\begin{matrix}1}\\end{matrix}").toFailWithParseError(
expect`{\begin{matrix}1}\end{matrix}`.toFailWithParseError(
"Expected & or \\\\ or \\cr or \\end at position 17:" +
" …\\begin{matrix}1}̲\\end{matrix}");
});
Expand All @@ -249,7 +249,7 @@ describe("environments.js:", function() {
describe("array environment", function() {
it("rejects unknown column types", function() {
// TODO: The error position here looks strange
expect("\\begin{array}{cba}\\end{array}").toFailWithParseError(
expect`\begin{array}{cba}\end{array}`.toFailWithParseError(
"Unknown column alignment: b at position 16:" +
" \\begin{array}{cb̲a}\\end{array}");
});
Expand All @@ -261,20 +261,20 @@ describe("functions.js:", function() {

describe("delimiter functions", function() {
it("reject invalid opening delimiters", function() {
expect("\\bigl 1 + 2 \\bigr").toFailWithParseError(
expect`\bigl 1 + 2 \bigr`.toFailWithParseError(
"Invalid delimiter: '1' after '\\bigl' at position 7:" +
" \\bigl 1̲ + 2 \\bigr");
});
it("reject invalid closing delimiters", function() {
expect("\\bigl(1+2\\bigr=3").toFailWithParseError(
expect`\bigl(1+2\bigr=3`.toFailWithParseError(
"Invalid delimiter: '=' after '\\bigr' at position 15:" +
" \\bigl(1+2\\bigr=̲3");
});
});

describe("\\begin and \\end", function() {
it("reject invalid environment names", function() {
expect("\\begin x\\end y").toFailWithParseError(
expect`\begin x\end y`.toFailWithParseError(
"Invalid environment name at position 8: \\begin x̲\\end y");
});
});
Expand All @@ -297,23 +297,23 @@ describe("Lexer:", function() {

describe("#_innerLexColor", function() {
it("reject hex notation without #", function() {
expect("\\textcolor{1a2b3c}{foo}").toFailWithParseError(
expect`\textcolor{1a2b3c}{foo}`.toFailWithParseError(
"Invalid color: '1a2b3c'" +
" at position 12: \\textcolor{1̲a̲2̲b̲3̲c̲}{foo}");
});
});

describe("#_innerLexSize", function() {
it("reject size without unit", function() {
expect("\\rule{0}{2em}").toFailWithParseError(
expect`\rule{0}{2em}`.toFailWithParseError(
"Invalid size: '0' at position 7: \\rule{0̲}{2em}");
});
it("reject size with bogus unit", function() {
expect("\\rule{1au}{2em}").toFailWithParseError(
expect`\rule{1au}{2em}`.toFailWithParseError(
"Invalid unit: 'au' at position 7: \\rule{1̲a̲u̲}{2em}");
});
it("reject size without number", function() {
expect("\\rule{em}{2em}").toFailWithParseError(
expect`\rule{em}{2em}`.toFailWithParseError(
"Invalid size: 'em' at position 7: \\rule{e̲m̲}{2em}");
});
});
Expand Down