Skip to content

fix(framework): Fix serverapp heartbeat simulation assertions for shared app process#6863

Closed
psfoley wants to merge 8 commits intomainfrom
heartbeat-test-fix
Closed

fix(framework): Fix serverapp heartbeat simulation assertions for shared app process#6863
psfoley wants to merge 8 commits intomainfrom
heartbeat-test-fix

Conversation

@psfoley
Copy link
Copy Markdown
Member

@psfoley psfoley commented Mar 27, 2026

No description provided.

…ared app process

Signed-off-by: Patrick Foley <patrick@flower.ai>
Copilot AI review requested due to automatic review settings March 27, 2026 04:31
@psfoley psfoley changed the title fix(e2e): make serverapp heartbeat simulation assertions compatible with shared app process fix(e2e): makes serverapp heartbeat simulation assertions compatible with shared app process Mar 27, 2026
psfoley and others added 2 commits March 26, 2026 21:39
@psfoley psfoley changed the title fix(e2e): makes serverapp heartbeat simulation assertions compatible with shared app process fix(framework): makes serverapp heartbeat simulation assertions compatible with shared app process Mar 27, 2026
@psfoley psfoley changed the title fix(framework): makes serverapp heartbeat simulation assertions compatible with shared app process fix(framework): Makes serverapp heartbeat simulation assertions compatible with shared app process Mar 27, 2026
@psfoley psfoley changed the title fix(framework): Makes serverapp heartbeat simulation assertions compatible with shared app process fix(framework): Fix serverapp heartbeat simulation assertions for shared app process Mar 27, 2026
@github-actions github-actions bot added the Maintainer Used to determine what PRs (mainly) come from Flower maintainers. label Mar 27, 2026
Copy link
Copy Markdown
Contributor

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

Updates the ServerApp heartbeat E2E test to accommodate simulation environments where multiple runs may share the same underlying app process, affecting expected terminal statuses after killing an app process.

Changes:

  • Capture all matching app-process PID(s) after the first run starts instead of a single PID.
  • Delay selecting the PID to kill until after both runs are confirmed running.
  • Relax run2 terminal-status assertions in simulation mode to allow FAILED as an acceptable outcome.

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

jafermarq and others added 5 commits March 27, 2026 09:50
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@psfoley
Copy link
Copy Markdown
Member Author

psfoley commented Mar 27, 2026

No longer necessary after #6858

@psfoley psfoley closed this Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintainer Used to determine what PRs (mainly) come from Flower maintainers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants