-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Producing unstable snapshots #5681
Comments
The issue can easily be detected (needs possibly several retries) with the following property based test: import fc from 'fast-check'
describe('objDisplay', () => {
test('Only produce valid strings with objDisplay', () => {
// NOTE: The instancitation of fc.anything(...) should probably be made shorter, today it does not provide any unicode-aware
// built-in version and thus require users to be verbose... Ideally, it should be as simple as `fc.anything({withUnicode: true})`
fc.assert(fc.property(fc.anything({ key: fc.fullUnicodeString(), values: [
fc.boolean(),
fc.maxSafeInteger(),
fc.double(),
fc.fullUnicodeString(),
fc.oneof(fc.fullUnicodeString(), fc.constant(null), fc.constant(undefined)),
] }), fc.integer({ min: 2 }), (value, truncate) => {
expect(() => encodeURI(objDisplay(value, { truncate }))).not.toThrow()
}))
})
})
// FAIL |threads| test/utils.spec.ts > objDisplay > Only produce valid strings with objDisplay
// Error: Property failed after 74 tests
// { seed: -1289013075, path: "73:1:1:2:2:2:4:3:2:2:3:2:2:2:3:2:2:2:6:5:5:7:5:6:6:6:9:7:6:7:0", endOnFailure: true }
// Counterexample: ["𐀀 ",4]
// Shrunk 30 time(s)
// Got AssertionError: expected [Function] to not throw an error but 'URIError: URI malformed' was thrown If adding tests (waiting your go), I'll probably not use property based technique for now to avoid pulling something additional to your dev-deps. But I wanted to let you know that it could help to detect such issues in an automatic fashion. So I can work on adding some if you feel it might be interested (Jest found many problems thanks to fast-check). |
@sheremet-va I'm working on a fix on loupe |
I just opened a PR on loupe's side (see chaijs/loupe#79). If merged the fix will change the behavior of Once merged on their side (if merged), I'll try to propose some tests on Vitest's side if you're ok with it. |
It should be fixed with the next version of loupe. My fix has just been merged on their side. Let me know if you want me to cover the issue with tests so that it gets caught next time |
It would be nice to have a test case for this in Vitest repo. You can add your tests somewhere to |
Will do! |
I just opened a PR fixing the issue by pulling the latest version of loupe. It also comes with relevant tests. See #5724 |
Describe the bug
The name of the test printed in the snapshot file may include wrongly-formed characters. On windows, it leads Vitest to consider the snapshot as obsolete and recreate a new one for each run.
Where do the bug comes from? -> loupe
The bug is visible in Vitest but comes from loupe and has been reported on their side too at chaijs/loupe#78.
In Vitest the call to loupe responsible to the problem is in
@vitest/utils/dist/chunk-display.js
withinobjDisplay
. Once fixed on loupe's side I can probably help and come with some dedicated tests covering that issue to avoid it to come back.Temporary fix
While waiting for a fix on loupe's side, I can propose a fix on your side that will drop/replace surrogates not being in pairs? What do you think?
Reproduction
Consider the following example:
It leads to the snapshot-ed value:
This snapshot name is invalid as it comes with a split-cat or more precisely a split-surrogate-pair which is not an existing character outside of JavaScript.
System Info
Used Package Manager
npm
Validations
The text was updated successfully, but these errors were encountered: