japa / snapshot

Snapshot plugin for Japa
MIT License
2 stars 1 forks source link

Snapshot for JSON file prints unusable output #4

Closed xmajzel closed 1 month ago

xmajzel commented 5 months ago

Package version

2.0.5

Describe the bug

Bug description:

Case 1 - file snapshot: Note - using Math.random() for book.rating, so it is always different, so the snapshot fails (after the first one is produced).

  test('json snapshot', ({ assert }) => {
    const book = {
      id: '1',
      title: 'some-book',
      rating: Math.random(),
    }

    assert.snapshot(book).match()
  })

Produced output:

...
  Assertion Error: 'Snapshot file > json snapshot 1' snapshot comparison failed

- Expected  - 5
+ Received  + 5

- {
-   "id": "1",
-   "rating": 0.1091535569921307,
-   "title": "some-book",
- }
+ "{
+   \"id\": \"1\",
+   \"rating\": 0.6270380896300971,
+   \"title\": \"some-book\",
+ }"

   at Object.match src/integrations/assert.ts:16
...

Case 2 - inline snapshot:

  test('inline snapshot', ({ assert }) => {
    const book = {
      id: '1',
      title: 'some-book',
      rating: Math.random(),
    }
    assert.snapshot(JSON.stringify(book)).matchInline(JSON.stringify(book))
  }).pin()

Produced output:

...
  Assertion Error: 'inline snapshot' snapshot comparison failed

- Expected  - 1
+ Received  + 1

- {"id":"1","title":"some-book","rating":0.6508982624640216}
+ "{\"id\":\"1\",\"title\":\"some-book\",\"rating\":0.6508982624640216}"

   at Object.matchInline src/integrations/assert.ts:32
...

Expected behaviour:

For the Case 1, I would like, that it correctly show Number of changed lines as well as correct encoding (without double stringifying object - same for Case 2), e.g. how Jest does it (https://jestjs.io/docs/snapshot-testing#updating-snapshots):

Jest snapshot

Reproduction repo

No response

stale[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

xmajzel commented 3 months ago

Hey, any updates on this? All needed to fix this is to remove double serialization in src/snapshot_file.ts as follows:

  /**
   * Returns the data needed for a future assertion
   */
  getSnapshotTestData(test: Test, value: any) {
    const snapshotName = this.getSnapshotName(test)

    if (!this.#cachedContent) {
      throw new Error(`Snapshot file ${this.#snapshotPath} not found`)
    }

    this.incrementTestCounter(test)

    const expected = prepareExpected(this.#cachedContent[snapshotName])
    const received = serializeSnapshotValue(value, this.#options.prettyFormatOptions)

    return {
      snapshotName,
-     expected: prepareExpected(this.#cachedContent[snapshotName]),
+    expected,
-     received: serializeSnapshotValue(received, this.#options.prettyFormatOptions),
+    received,
      pass: expected === received,
    }
  }

Can provide PR if needed 😄

xmajzel commented 3 months ago

@Julien-R44

xmajzel commented 3 months ago

@thetutlage

stale[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Julien-R44 commented 1 month ago

Sorry for the super late reply, especially as the fix was indeed super simple :/ New release is coming soon