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

Use Snapshot Testing for WASM Inference Tests #946

Open
nordzilla opened this issue Nov 26, 2024 · 3 comments
Open

Use Snapshot Testing for WASM Inference Tests #946

nordzilla opened this issue Nov 26, 2024 · 3 comments

Comments

@nordzilla
Copy link
Contributor

We now have several test files in inference/wasm/tests/test-cases that compare before-and-after translations.

It would be a good idea to transition these test cases to use vitest's Snapshot capabilities, so that it is easy to change expected assertions when adding new models, or if models change, etc.

@nordzilla nordzilla changed the title Use Snapshot Testing for testing model inference Use Snapshot Testing for Testing Model Inference Nov 26, 2024
@nordzilla nordzilla changed the title Use Snapshot Testing for Testing Model Inference Use Snapshot Testing for Model Inference Tests Nov 27, 2024
@nordzilla nordzilla changed the title Use Snapshot Testing for Model Inference Tests Use Snapshot Testing for WASM Inference Tests Nov 27, 2024
@gregtatum
Copy link
Member

My preference from other projects is to use inline snapshots if possible. It makes it easier to review and maintain tests. They are a bit more magical though.

@nordzilla
Copy link
Contributor Author

I would say that inline snapshots via data-driven test assertions is effectively what I've already implemented.

Does that mean that you're overall fine with the current format?

@gregtatum
Copy link
Member

Yeah I'm fine with how things are implemented, but it won't work with the matchInlineSnapshot feature, as that rewrites the files in place:

import { expect, it } from 'vitest'

it('toUpperCase', () => {
  const result = toUpperCase('foobar')
  expect(result).toMatchInlineSnapshot('"FOOBAR"')
})

The benefits of snapshot testing is that the tests can be updated through an automated command as opposed to manually updating the examples yourself. I have found non-inline snapshot hard to use in the past as the expectations are separated from the actual results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants