Skip to content

Conversation

@lopert
Copy link
Contributor

@lopert lopert commented Oct 5, 2025

Background

An updated version of #321
Fixes https://github.com/shop/issues-shopifyvm/issues/552 and https://github.com/shop/issues-shopifyvm/issues/550

Solution

Adds testing files to function templates in order to enable wasm-module testing out of the box.

Tophat

You can run these changes with the following setup:

  • You'll need a version of the shopify CLI that has the new info command.
npm i -g @shopify/cli@latest
  • If needed, init a shopify app.
shopify app init --template=none
  • Generate a new function, targeting this branch
shopify app generate extension -u https://github.com/Shopify/extensions-templates.git\#lopert.add-wasm-tests
  • Confirm that the newly generated function has the tests directory, containing a default test, and a basic fixtures/log.json.

  • If you made a rust function, you'll need to npm install in the root of your app so that the devDepencies from the package.json get installed. This should happen automatically for JS functions.

  • You should just be able to run

    • npx vitest run from the root of your app, or
    • npm test from your function's directory.

Checklist

  • I have 🎩'd these changes
  • I have squashed my commits into chunks of work with meaningful commit messages

@lopert lopert force-pushed the lopert.add-wasm-tests branch from ba81395 to 3b6dea4 Compare October 6, 2025 19:54
@lopert lopert marked this pull request as ready for review October 15, 2025 15:56
Copy link

@saga-dasgupta saga-dasgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't tophat all the fixtures but if they were copied from #321 then they should all work because I had tophatted those.
The default.test and package.json looks good.

Copy link

@saga-dasgupta saga-dasgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should pass the entire fixture to runFunction, then if we need more components from the fixture in future to run a function, we dont have to change method signatures.

@lopert lopert force-pushed the lopert.add-wasm-tests branch from dfa9683 to db32d7f Compare October 22, 2025 19:07
@lopert lopert requested a review from saga-dasgupta November 5, 2025 20:50
"test": "vitest run"
},
"devDependencies": {
"@shopify/shopify-function-test-helpers": "~0.0.0",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this version be something like 1.0.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be, once the package is released at 1.0.0.
For now, the package is 0.0.2, and in order to get the extension generating command and follow up test commands running smoothly, I need to target these pre-release versions. So for tophatting at the moment, this is correct.

I will leave this comment open as a reminder that this needs to be flipped over to 1.0.0 once we fully release.

@lopert lopert force-pushed the lopert.add-wasm-tests branch from 2837550 to 010f643 Compare November 6, 2025 15:40
@lopert lopert requested a review from saga-dasgupta November 6, 2025 15:49
@lopert lopert force-pushed the lopert.add-wasm-tests branch from 76faa7a to 0c1a210 Compare November 6, 2025 16:41
Copy link

@saga-dasgupta saga-dasgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the second discount target fixture and addressing all the comments, I tophatted this for both discount-js and discount-rs and sometimes the setup step for build and info commands times out for the rust function so we might wanna increase that timeout.

@lopert lopert changed the title Add wasm tests [DNM] Add wasm level integration tests to function templates Nov 7, 2025
@lopert lopert force-pushed the lopert.add-wasm-tests branch 2 times, most recently from baa420c to a8f02b1 Compare November 7, 2025 20:43
@lopert lopert force-pushed the lopert.add-wasm-tests branch from a8f02b1 to fb52ce1 Compare December 2, 2025 14:17
@lopert lopert changed the title [DNM] Add wasm level integration tests to function templates Add wasm level integration tests to function templates Dec 2, 2025
@lopert lopert requested a review from saga-dasgupta December 2, 2025 14:30
Copy link

@saga-dasgupta saga-dasgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run: yarn js-typegen
- name: Test
run: yarn js-test
run: yarn js-test:unit

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with this command why did this have to change?

Copy link
Contributor Author

@lopert lopert Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc this was so that CI wouldn't run the integration tests.
I dont recall what the exact errors were, but I think it had something to do with installing/using the CLI

I'll try flipping this back and seeing what comes back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, since this repo doesn't depend on the CLI (and function-runner), we can't really use the shopify commands to run the integration tests.

https://github.com/Shopify/extensions-templates/actions/runs/19898271656/job/57034488324

@lopert lopert force-pushed the lopert.add-wasm-tests branch from 8199e8b to a99d8df Compare December 3, 2025 15:25
@lopert
Copy link
Contributor Author

lopert commented Dec 3, 2025

Chatted on slack. We'll keep this PR as-is, focused on adding the new tests.
I will create a follow up PR focused on removing / converting existing use cases to additional tests/fixtures.

@lopert lopert merged commit 596019c into main Dec 3, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants