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.

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

assertSnapshot fails to match carriage return #4226

Closed
Tracked by #5010
OzzieOrca opened this issue Jan 21, 2024 · 5 comments · Fixed by #5352
Closed
Tracked by #5010

assertSnapshot fails to match carriage return #4226

OzzieOrca opened this issue Jan 21, 2024 · 5 comments · Fixed by #5352
Labels
bug Something isn't working PR welcome A pull request for this issue would be welcome testing

Comments

@OzzieOrca
Copy link

OzzieOrca commented Jan 21, 2024

Describe the bug
If you pass the CR character \r to assertSnapshot, it saves the snapshot fine but then immediately fails to match the snapshot.

I ran into this by trying to snapshot the output of stringify(...) from https://deno.land/[email protected]/csv/mod.ts which apparently output CRLFs (\r\n).

Steps to Reproduce

  1. Create a file (crlf.test.ts) with the contents
    import { assertSnapshot } from 'https://deno.land/[email protected]/testing/snapshot.ts';
    
    Deno.test('CR failure', async (t) => {
      await assertSnapshot(t, '\r');
    });
  2. Run deno test --allow-read --allow-write cr.test.ts -- --update which returns
    running 1 test from ./cr.test.ts
    CR failure ... ok (2ms)
    ------- output -------
    
    > 1 snapshot updated.
    
    ok | 1 passed | 0 failed (4ms)
    
    and saves __snapshots__/cr.test.ts.snap with
    export const snapshot = {};
    
    snapshot[`CR failure 1`] = `
    "
    "
    `;
  3. Run deno test --allow-read cr.test.ts which returns:
    running 1 test from ./cr.test.ts
    CR failure ... FAILED (6ms)
    
     ERRORS 
    
    CR failure => ./cr.test.ts:3:6
    error: AssertionError: Snapshot does not match:
    
    
        [Diff] Actual / Expected
    
    
    -   "
    +   "
        "
    
        throw new AssertionError(
              ^
        at assertSnapshot (https://deno.land/[email protected]/testing/snapshot.ts:591:11)
        at async file:///home/scotty/code/Toptal/FoodNoms/macrosai-supabase/cr.test.ts:4:3
    
     FAILURES 
    
    CR failure => ./cr.test.ts:3:6
    
    FAILED | 0 passed | 1 failed (8ms)
    
    error: Test failed
    

Expected behavior

The test should succeed after the snapshot has been updated.

Environment

  • OS: Ubuntu 22.04.3 LTS
  • deno version: 1.39.1
  • std version: 0.212.0
@OzzieOrca OzzieOrca added bug Something isn't working needs triage labels Jan 21, 2024
@kt3k kt3k removed the needs triage label Jan 21, 2024
@iuioiua iuioiua added the PR welcome A pull request for this issue would be welcome label Jan 22, 2024
@javihernant
Copy link
Contributor

I'm taking a look at this one

@javihernant
Copy link
Contributor

javihernant commented Feb 23, 2024

The culprit seems to be here. When .ts.snap is written, an actual "\r" is written to the file. However, when importing it reads a "\n". import bugs have to be fixed in the actual runtime (not in deno_std), right? Note: this one was fun to find! ^^

@kt3k
Copy link
Member

kt3k commented Feb 24, 2024

@javihernant Interesting finding! Can you create an issue in denoland/deno with a minimal repro example?

@kt3k
Copy link
Member

kt3k commented Mar 16, 2024

hmm looks like Deno ignoring \r in string literal is not a bug. Maybe we should escape \r when serializing it

@OzzieOrca
Copy link
Author

Sounds like assertSnapshot should strip (or escape somehow?) \r characters before comparing and storing snapshots. At least that's my takeaway from denoland/deno#22575 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR welcome A pull request for this issue would be welcome testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants