Skip to content

Stabilize runTrampoline unit tests#7101

Open
dmerand wants to merge 2 commits intomainfrom
dlm-flaky-tests-redux
Open

Stabilize runTrampoline unit tests#7101
dmerand wants to merge 2 commits intomainfrom
dlm-flaky-tests-redux

Conversation

@dmerand
Copy link
Contributor

@dmerand dmerand commented Mar 25, 2026

What

Stabilize the runTrampoline unit tests in packages/app/src/cli/services/function/build.test.ts.

Why

main hit a timeout in runTrampoline > runs v1 trampoline on v1 module in 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

  • use a fixed wasm module path in the runTrampoline tests instead of constructing a full function extension
  • mock binary existence checks so the tests skip download setup
  • keep coverage on wasm import detection and the expected trampoline invocation

Testing

Re-run the previously failing macOS Node 20 CI lane and confirm it stays green.

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@dmerand dmerand marked this pull request as ready for review March 25, 2026 18:40
@dmerand dmerand requested a review from a team as a code owner March 25, 2026 18:40
Copilot AI review requested due to automatic review settings March 25, 2026 18:40
Copy link
Contributor Author

dmerand commented Mar 25, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2026

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 82.28% 15106/18360
🟡 Branches 74.85% 7453/9957
🟢 Functions 81.31% 3793/4665
🟢 Lines 82.67% 14283/17278

Test suite run success

3990 tests passing in 1527 suites.

Report generated by 🧪jest coverage report action from 9d01c5e

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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().outputPath usage in runTrampoline tests with a shared functionModulePath constant.
  • Adds fileExists to the fs imports and mocks it to resolve true in beforeEach.

💡 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'
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@dmerand dmerand marked this pull request as draft March 25, 2026 18:50
@dmerand dmerand force-pushed the dlm-flaky-tests-redux branch from 134579d to 9d01c5e Compare March 26, 2026 16:39
@dmerand dmerand marked this pull request as ready for review March 26, 2026 17:48
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