Improves the terse messages in jest-editor #3615
Conversation
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "jest.pathToJest": "npm run jest --", | |||
| "jest.pathToJest": "yarn jest --", | |||
There was a problem hiding this comment.
Works nicely, but it should be yarn to fit with the rest of the dev recommendations
| if (string.includes("New snapshot was not written")) { | ||
| return "New snapshot is ready to write" | ||
| } | ||
|
|
There was a problem hiding this comment.
These are some special cases, you can't show anything of value for these two cases in a tweet-sized message, so I've added them as special cases.
| .replace('Difference:', ' Difference:'); | ||
| .replace(/\s\s+/g, ' ') | ||
| .replace('Received:', ', Received:') | ||
| .split('Difference:')[0]; |
There was a problem hiding this comment.
Turned out the two replaces above had some invisible control characters too:
| message = "Expected value to be truthy, instead received null" | ||
| testName = "truthy" | ||
| expect(terseForTest(testName).terseMessage).toEqual(message) | ||
| }) |
There was a problem hiding this comment.
didn't think it was worth having all the work split across multiple its - it slows down the tests needlessly with all the JSON parsing + string manipulation.
| @@ -108,15 +108,24 @@ module.exports = class TestReconciler { | |||
|
|
|||
| // Do everything we can to try make a one-liner from the error report | |||
| sanitizeShortErrorMessage(string: string): string { | |||
| if (string.includes("does not match stored snapshot")) { | |||
There was a problem hiding this comment.
We need more structured data around this so that it won't break going forward.
There was a problem hiding this comment.
I had a look at this, but the way things are pieced together is like this:
There is a this test which incidentally covers this case, I could add comments to it?
There was a problem hiding this comment.
Yeah, adding a comment for now would be very nice.
| @@ -52,3 +52,41 @@ Expected value to be falsy, instead received | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('Terse Messages', () => { | |||
| it("handles shrinking a snapshot message", () => { | |||
There was a problem hiding this comment.
single quote please (did you run prettier on this file?)
There was a problem hiding this comment.
It looks like it wasn't set up to run, I've fixed that - good catch.
…_terse_messages
…the error it is implicity relying on
|
@orta mind making sure that |
…the error it is implicity relying on
orta
force-pushed
the
better_terse_messages
branch
from
5c1e572 to
8359083
Compare
|
🥗 |
* Improves the terse messages in jest-editor * Adds a note about wording for jest-editor inside a test that matches the error it is implicity relying on * Adds a note about wording for jest-editor inside a test that matches the error it is implicity relying on
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
The terse messages sometimes were not terse, as they're built for showing inline they definitely needed cutting down to size in some places. So I made a list of some common failing assertions and added some tests against each of them.
I turned the results of this into a fixture, and started working from that.
Test plan
This has unit tests 👍