Conversation
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
Coverage report
Test suite run success3990 tests passing in 1527 suites. Report generated by 🧪jest coverage report action from 9d01c5e |
There was a problem hiding this comment.
Pull request overview
This PR aims to stabilize the runTrampoline unit tests in the function build test suite by removing reliance on dynamically created testFunctionExtension() instances and using a consistent module path/mocking setup.
Changes:
- Replaces per-test
testFunctionExtension().outputPathusage inrunTrampolinetests with a sharedfunctionModulePathconstant. - Adds
fileExiststo the fs imports and mocks it to resolvetrueinbeforeEach.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import {exec} from '@shopify/cli-kit/node/system' | ||
| import {dirname, joinPath} from '@shopify/cli-kit/node/path' | ||
| import {inTemporaryDirectory, mkdir, readFileSync, writeFile, removeFile} from '@shopify/cli-kit/node/fs' | ||
| import {fileExists, inTemporaryDirectory, mkdir, readFileSync, writeFile, removeFile} from '@shopify/cli-kit/node/fs' |
There was a problem hiding this comment.
fileExists is imported here and later mocked in beforeEach, but the functions under test in this file (including runTrampoline) don’t call fileExists (they use readFileSync). Consider removing the fileExists import and its mock setup unless you’re intentionally asserting behavior that depends on it.
134579d to
9d01c5e
Compare

What
Stabilize the
runTrampolineunit tests inpackages/app/src/cli/services/function/build.test.ts.Why
mainhit a timeout inrunTrampoline > runs v1 trampoline on v1 modulein this broken build. These tests were doing more setup than the code under test needs, which made them slower and more prone to flake.How
runTrampolinetests instead of constructing a full function extensionTesting
Re-run the previously failing macOS Node 20 CI lane and confirm it stays green.
Measuring impact
Checklist