Skip to content

Add test for --unload-on-kill spawner lock behavior#3035

Open
naitikpahwa18 wants to merge 4 commits intoros-controls:masterfrom
naitikpahwa18:test-unload-on-kill-2566
Open

Add test for --unload-on-kill spawner lock behavior#3035
naitikpahwa18 wants to merge 4 commits intoros-controls:masterfrom
naitikpahwa18:test-unload-on-kill-2566

Conversation

@naitikpahwa18
Copy link

This PR adds a regression test to verify that a spawner started with
--unload-on-kill does not block other spawners while waiting for an
interrupt.

The test launches one spawner in the background and ensures a second
spawner can start without being delayed, covering the locking fix
introduced in #2559

Closes #2566

Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.com>
Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.com>
Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.com>
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

Adds a C++ regression test in controller_manager to ensure a spawner started with --unload-on-kill does not hold the global spawner lock while waiting for interrupt, allowing other spawners to start promptly (covers the fix from #2559; closes #2566).

Changes:

  • Add unload_on_kill_does_not_block_other_spawners test that runs one spawner in the background with --unload-on-kill.
  • Measure that a second spawner invocation completes within a bounded time, indicating it wasn’t blocked on the lock.

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

Signed-off-by: Naitik Pahwa <naitikpahwa18@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.43%. Comparing base (c569292) to head (d955886).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
controller_manager/test/test_spawner_unspawner.cpp 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3035      +/-   ##
==========================================
+ Coverage   89.39%   89.43%   +0.03%     
==========================================
  Files         157      157              
  Lines       18783    18794      +11     
  Branches     1510     1511       +1     
==========================================
+ Hits        16792    16808      +16     
+ Misses       1369     1366       -3     
+ Partials      622      620       -2     
Flag Coverage Δ
unittests 89.43% <91.66%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
controller_manager/test/test_spawner_unspawner.cpp 96.00% <91.66%> (-0.13%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Add Spawner test for the --unload-on-kill locking other spawners

3 participants