Restore test coverage for “x-parser.js”. #284
Conversation
In a prior change set, we changed our `Map` error mappings to `switch` error mappings so that our code coverage tests would ensure that we are able to prove that each error is possible to be shown — otherwise, why keep them. This change set was kept separate since it’s all about coverage. Note that some of the regular expressions have changed — nearly all of that is due to rewriting like `\s\n` as just `\s` since `\n` is one of the characters matched by the `\s` set.
| @@ -7,7 +7,7 @@ import '../x-template.js'; | |||
|
|
|||
| // Set a high bar for code coverage! | |||
| coverage(new URL('../x-element.js', import.meta.url).href, 100); | |||
| coverage(new URL('../x-parser.js', import.meta.url).href, 97); // TODO: Get back to 100%! | |||
| coverage(new URL('../x-parser.js', import.meta.url).href, 100); | |||
There was a problem hiding this comment.
And we’re back 🎉
| static #endTag = /<\/(?![0-9-])[a-z0-9-]+(?<!-)>/y; | ||
| static #startTagClose = /(?<![\s\n])>/y; |
There was a problem hiding this comment.
There’s no reason to write [\s\n] — \s includes \n.
| @@ -113,7 +111,7 @@ export class XParser { | |||
| // Examples: | |||
| // - ok: <div foo bar>, <div\n foo\n bar> | |||
| // - not ok: <div foo bar>, <div\n\n foo\n\n bar>, <div\tfoo\tbar> | |||
| static #startTagSpace = / (?! )|\n *(?!\n)(?=[-_.?a-zA-Z0-9>])/y; | |||
There was a problem hiding this comment.
I was able to simplify this by adding a new invalid state transition — namely, we now check to see if a space is followed by an invalid (or duplicate) space. Strictly-speaking, removing the lookahead should eek out minor performance gains as well (although I didn’t scrutinize that this round).
| @@ -239,61 +237,57 @@ export class XParser { | |||
| case '#101': return 'Could not parse template markup (after text content).'; | |||
| case '#102': return 'Could not parse template markup (after a comment).'; | |||
| case '#103': return 'Could not parse template markup (after interpolated content).'; | |||
| case '#104': return 'Could not parse template markup (after a start tag name).'; | |||
There was a problem hiding this comment.
Because some of these were deemed impossible to hit — I removed them and did some reordering of the error keys.
| @@ -382,6 +377,7 @@ export class XParser { | |||
| case XParser.#danglingQuote: return XParser.#try(string, stringIndex, | |||
| XParser.#startTagSpaceMalformed); | |||
| case XParser.#startTagSpace: return XParser.#try(string, stringIndex, | |||
| XParser.#startTagSpaceMalformed, | |||
There was a problem hiding this comment.
Again, this is the new invalid state transition that was added to decrease the complexity of the #startTagSpace pattern.
| case XParser.#boundProperty: { | ||
| const errorMessageKey = XParser.#getErrorMessageKeyFromErrorName('missing-closing-quote'); | ||
| const errorMessage = XParser.#getErrorMessage(errorMessageKey); | ||
| const substringMessage = `Missing closing double-quote.`; |
There was a problem hiding this comment.
As part of testing — I found a new case that we had poor messaging on. When you are not done writing your template, you can hit this issue:
html`<div foo="${bar}`; // Note the missing double-quote at the end.
| if (tagName) { | ||
| const errorMessageKey = XParser.#getErrorMessageKeyFromErrorName('missing-closing-tag'); | ||
| const errorMessageKey = XParser.#getErrorMessageKeyFromErrorName('missing-end-tag'); |
There was a problem hiding this comment.
Making our language consistent — missed this one last round!
| }); | ||
|
|
||
| // Sanity check to make sure we’re able to hit all our errors. | ||
| describe('errors coverage', () => { |
There was a problem hiding this comment.
Added a dedicated errors coverage test section where we can add odds-and-ends tests which are required to meet our 100% coverage goals.
| // - XParser.#startTagOpen (caught by XParser.#startTagSpaceMalformed) | ||
| // - XParser.#boolean (caught by XParser.#startTagSpaceMalformed) | ||
| // - XParser.#attribute (caught by XParser.#startTagSpaceMalformed) | ||
| // - XParser.#danglingQuote (caught by XParser.#startTagSpaceMalformed) |
There was a problem hiding this comment.
Added commentary as to why the list below doesn’t include all the value states. Kinda neat that we were able to get our code coverage tool to find unreachable cases for us 🎉
|
@klebba — this may be one of the more boring change sets that I’ve committed in recent history… but… we’re back up to |
In a prior change set, we changed our
Maperror mappings toswitcherror mappings so that our code coverage tests would ensure that we are able to prove that each error is possible to be shown — otherwise, why keep them.This change set was kept separate since it’s all about coverage.
Note that some of the regular expressions have changed — nearly all of that is due to rewriting like
\s\nas just\ssince\nis one of the characters matched by the\sset.