Skip to content

Make StopAgent synchronous in integration tests#449

Open
ystros wants to merge 1 commit into
mainfrom
make-stop-synchronous
Open

Make StopAgent synchronous in integration tests#449
ystros wants to merge 1 commit into
mainfrom
make-stop-synchronous

Conversation

@ystros

@ystros ystros commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Currently, the integration tests manually call StopAgent + StartAgent or RestartAgent in various spots. StopAgent was backgrounding the underlying sv stop call, meaning it wouldn't necessarily wait until the bosh-agent service was stopped before executing StartAgent. This race condition could potentially be the cause of some of the flakes we see in CI.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ystros, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 199007c5-5f1a-4cff-b7e8-0856b214224b

📥 Commits

Reviewing files that changed from the base of the PR and between 56e5607 and 40f8af6.

📒 Files selected for processing (1)
  • integration/test_environment.go

Walkthrough

TestEnvironment.StopAgent() now reads BOSH_AGENT_SV_STOP_TIMEOUT, defaults it to 30 when unset, and stops the agent with sv -w <timeout> stop agent. It continues returning the error from RunCommand.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change and motivation, but it omits most required template sections like tests, release notes, breaking changes, and tags. Add the missing template sections: context, tests run, release note wording, breaking change status, tags, and AI review feedback.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: making StopAgent synchronous in integration tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch make-stop-synchronous

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@integration/test_environment.go`:
- Around line 606-610: Validate BOSH_AGENT_SV_STOP_TIMEOUT in the test cleanup
flow before constructing the sv command: parse it as a positive integer, fall
back to the existing 30-second default when unset or invalid, and pass the
validated numeric value through a safe command-argument mechanism rather than
interpolating the raw environment string into RunCommand.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ccd7205-24b4-47e8-b266-f7b6833905b7

📥 Commits

Reviewing files that changed from the base of the PR and between 85b6ed8 and 56e5607.

📒 Files selected for processing (1)
  • integration/test_environment.go

Comment thread integration/test_environment.go Outdated
Currently, the integration tests manually call StopAgent + StartAgent or
RestartAgent in various spots. StopAgent was backgrounding the underlying `sv
stop` call, meaning it wouldn't necessarily wait until the bosh-agent service
was stopped before executing StartAgent. This race condition could potentially
be the cause of some of the flakes we see in CI.
@ystros
ystros force-pushed the make-stop-synchronous branch from 56e5607 to 40f8af6 Compare July 17, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending Merge | Prioritized

Development

Successfully merging this pull request may close these issues.

2 participants