Skip to content

Avoid re-connecting every Ctrl+C (bugfix)#2463

Open
Hook25 wants to merge 3 commits intomainfrom
fix_ctrl_c_menu
Open

Avoid re-connecting every Ctrl+C (bugfix)#2463
Hook25 wants to merge 3 commits intomainfrom
fix_ctrl_c_menu

Conversation

@Hook25
Copy link
Copy Markdown
Collaborator

@Hook25 Hook25 commented Apr 14, 2026

Description

This is a weird behaviour because regardless of what the user picks from the ctrl+c menu, we will always re-connect from scratch and sometimes mess up the session

image

Resolved issues

N/A (Ctrl+C didnt work, with this patch it does)

Documentation

N/A

Tests

Tested locally, before this patch, Ctrl+C always reconnects, so it basically never works.

This is a weird behaviour because regardless of what the user picks from
the ctrl+c menu, we will always re-connect from scratch and sometimes
mess up the session
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.87%. Comparing base (03581f9) to head (72d7c1c).
⚠️ Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
checkbox-ng/checkbox_ng/launcher/controller.py 0.00% 15 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2463   +/-   ##
=======================================
  Coverage   58.86%   58.87%           
=======================================
  Files         476      476           
  Lines       48007    48002    -5     
  Branches     8570     8568    -2     
=======================================
  Hits        28260    28260           
+ Misses      18855    18851    -4     
+ Partials      892      891    -1     
Flag Coverage Δ
checkbox-ng 76.05% <0.00%> (+0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@fernando79513
Copy link
Copy Markdown
Collaborator

Please add a more detailed description of what the behavior was before this PR, and how this solves the issue.
I tried to run smoke tests with the previous version with:

checkbox.checkbox-cli control 10.164.173.184 smoke_launcher 

And I was able to continue the execution, disconnect from the agent, and start a new one.

Copy link
Copy Markdown
Collaborator

@fernando79513 fernando79513 left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this.

I think this PR could benefit greatly from some metabox tests. I don't know how difficult it would be to add them, but I think they can help us avoid these kinds of issues in the future.

@Hook25 Hook25 force-pushed the fix_ctrl_c_menu branch 2 times, most recently from a6069c1 to cd38903 Compare April 17, 2026 14:49
@fernando79513
Copy link
Copy Markdown
Collaborator

I run the metabox tests manually yesterday and they are failing:
https://github.com/canonical/checkbox/actions/runs/24726686275/job/72329566224

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