Skip to content

[eas-cli] Stop simulator job when start is canceled - #4113

Merged
sjchmiela merged 4 commits into
mainfrom
stanley/eng-25545-stop-simulator-job-run-on-early-cancel
Jul 30, 2026
Merged

[eas-cli] Stop simulator job when start is canceled#4113
sjchmiela merged 4 commits into
mainfrom
stanley/eng-25545-stop-simulator-job-run-on-early-cancel

Conversation

@sjchmiela

@sjchmiela sjchmiela commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Fixes ENG-25545.

The SIGINT handler was registered only after the simulator became ready, so canceling earlier left the Turtle job running until it timed out. This registers one handler immediately after session creation and reuses it for the whole session lifecycle.

Added a regression test and tested it live in test-production: canceled while it was still waiting for readiness, the session never started (startedAt: null), and its final status was STOPPED.

Also ran the targeted tests, typecheck, lint, and formatter.

@linear-code

linear-code Bot commented Jul 29, 2026

Copy link
Copy Markdown

ENG-25545

@sjchmiela sjchmiela changed the title Stop simulator job when start is canceled [eas-cli] Stop simulator job when start is canceled Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.68966% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.32%. Comparing base (2e8cbb5) to head (42c0aa2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/eas-cli/src/commands/simulator/start.ts 70.22% 14 Missing ⚠️
packages/eas-cli/src/utils/promise.ts 72.73% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4113      +/-   ##
==========================================
+ Coverage   62.29%   62.32%   +0.03%     
==========================================
  Files         995      995              
  Lines       44888    44926      +38     
  Branches     9435     9442       +7     
==========================================
+ Hits        27960    27995      +35     
- Misses      15479    15483       +4     
+ Partials     1449     1448       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sjchmiela
sjchmiela marked this pull request as ready for review July 29, 2026 16:29
@sjchmiela
sjchmiela requested a review from szdziedzic July 29, 2026 16:29
@github-actions

Copy link
Copy Markdown

Subscribed to pull request

File Patterns Mentions
packages/eas-cli/** @douglowder

Generated by CodeMention

Warning: The preamble and epilogue options in commentConfiguration are deprecated. Use template instead.

Comment thread packages/eas-cli/src/commands/simulator/start.ts
Comment thread packages/eas-cli/src/commands/simulator/start.ts Outdated
@sjchmiela
sjchmiela force-pushed the stanley/eng-25545-stop-simulator-job-run-on-early-cancel branch from 4ec8064 to 1999f58 Compare July 30, 2026 14:50
@sjchmiela
sjchmiela requested a review from Copilot July 30, 2026 14:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes eas simulator:start leaving a remote Turtle job running when the command is canceled (Ctrl+C) before the simulator session becomes “ready”, by registering a SIGINT handler immediately after session creation and reusing it throughout the session lifecycle.

Changes:

  • Add optional AbortSignal support to sleepAsync so polling waits can end early on abort.
  • Register and reuse a session-scoped SIGINT/abort helper in simulator/start, ensuring early cancellation triggers a stop + cleanup.
  • Add regression tests for abortable sleep and for interrupting before readiness; update the root changelog entry.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/eas-cli/src/utils/promise.ts Extends sleepAsync to support aborting polling sleeps early via AbortSignal.
packages/eas-cli/src/utils/tests/promise.test.ts Adds a unit test for aborting sleepAsync and clearing its timer.
packages/eas-cli/src/commands/simulator/start.ts Introduces a reusable session interrupt handler and ensures early Ctrl+C stops the remote session.
packages/eas-cli/src/commands/simulator/tests/start.test.ts Adds a regression test ensuring interruption before readiness stops the session and cleans env.
CHANGELOG.md Documents the bug fix in the main changelog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/eas-cli/src/utils/promise.ts
Comment thread packages/eas-cli/src/utils/__tests__/promise.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

packages/eas-cli/src/commands/simulator/start.ts:188

  • Promise.race([DeviceRunSessionQuery.byIdAsync(...), sessionInterrupt.abortPromise]) can leave the byIdAsync promise in-flight after Ctrl+C abort wins the race. If that request later rejects (network/GraphQL error), it will be an unhandled rejection because nothing awaits/catches it, which can produce warnings or terminate the process depending on Node settings.

Consider attaching a .catch(...) to the query promise that rethrows when not aborted, but swallows the error once sessionInterrupt.signal.aborted is true, so late failures after an abort are handled.

      while (!sessionInterrupt.signal.aborted && Date.now() < deadline) {
        const session = await Promise.race([
          DeviceRunSessionQuery.byIdAsync(graphqlClient, deviceRunSessionId),
          sessionInterrupt.abortPromise,
        ]);

packages/eas-cli/src/commands/simulator/tests/start.test.ts:153

  • This suite uses jest.spyOn(process, 'exit') in a test, but currently relies on an explicit mockRestore() at the end of the test body. If an assertion throws before that line, the spy won’t be restored and can leak into subsequent tests. Adding a suite-level afterEach(() => jest.restoreAllMocks()) makes this cleanup reliable for all tests in this file.
  beforeEach(() => {
    jest.clearAllMocks();
    delete process.env[EAS_SIMULATOR_SESSION_ID];
    mockCreateDeviceRunSessionAsync.mockResolvedValue(makeCreatedDeviceRunSession());
    mockEnsureDeviceRunSessionStoppedAsync.mockResolvedValue({

@sjchmiela
sjchmiela force-pushed the stanley/eng-25545-stop-simulator-job-run-on-early-cancel branch from 15f4210 to 42c0aa2 Compare July 30, 2026 15:35
@sjchmiela
sjchmiela enabled auto-merge (squash) July 30, 2026 15:35
@github-actions

Copy link
Copy Markdown

✅ Thank you for adding the changelog entry!

@sjchmiela
sjchmiela merged commit f49e93d into main Jul 30, 2026
7 checks passed
@sjchmiela
sjchmiela deleted the stanley/eng-25545-stop-simulator-job-run-on-early-cancel branch July 30, 2026 15:38
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.

3 participants