Skip to content

Conversation

@gntzio
Copy link
Collaborator

@gntzio gntzio commented Jul 4, 2025

Description

  • Prevents crashing of the application when a test plan uses only hidden manifests - manifest selection screen will not be shown if test plan contains only hidden manifests.
  • For better unit testing and readability the logic of ManifestBrowser.unhandled_input is extracted into handle_submit_key & handle_focused_question_input.
  • Adds unit tests for ManifestBrowser to cover implemented logic.
  • Moves manifest-related logic from Launcher._save_manifest to ManifestBrowser - Launcher should work with manifests only through ManifestBrowser. The same done in RemoteController.
  • Extends Launcher & RemoteController tests with _save_manifest tests.

Resolved issues

Fixes: CHECKBOX-1963

Documentation

N/A

Tests

The fix was tested manually (only Launcher) + covered by unit tests (see the Description).

@gntzio gntzio requested a review from Hook25 July 4, 2025 05:55
@codecov
Copy link

codecov bot commented Jul 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.15%. Comparing base (dfbf93e) to head (0f9b3b8).
⚠️ Report is 98 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1996      +/-   ##
==========================================
+ Coverage   51.13%   51.15%   +0.02%     
==========================================
  Files         386      386              
  Lines       41603    41602       -1     
  Branches     7730     7727       -3     
==========================================
+ Hits        21272    21280       +8     
+ Misses      19569    19560       -9     
  Partials      762      762              
Flag Coverage Δ
checkbox-ng 70.78% <100.00%> (+0.05%) ⬆️

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.

@Hook25 Hook25 self-assigned this Jul 4, 2025
@gntzio gntzio force-pushed the manifest-edit-crash-hidden-only branch from c835e9b to 433be4f Compare July 4, 2025 15:00
Copy link
Collaborator

@Hook25 Hook25 left a comment

Choose a reason for hiding this comment

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

My main comment here is: dont duplicate the code to get manifests by visibility.

@gntzio gntzio requested a review from Hook25 July 10, 2025 14:08
@Hook25 Hook25 added the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Jul 10, 2025
@gntzio gntzio removed the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Jul 10, 2025
@Hook25 Hook25 added the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Jul 16, 2025
Hook25
Hook25 previously approved these changes Jul 21, 2025
Copy link
Collaborator

@Hook25 Hook25 left a comment

Choose a reason for hiding this comment

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

+1, thanks

please rebase your PR on main

@gntzio gntzio force-pushed the manifest-edit-crash-hidden-only branch from a014835 to 0f9b3b8 Compare July 21, 2025 15:16
@gntzio gntzio removed the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Jul 21, 2025
@gntzio gntzio requested a review from Hook25 July 21, 2025 15:19
@gntzio
Copy link
Collaborator Author

gntzio commented Jul 23, 2025

+1, thanks

please rebase your PR on main

@Hook25, Done.

@Hook25 Hook25 merged commit b2fd1b2 into main Jul 24, 2025
35 checks passed
@Hook25 Hook25 deleted the manifest-edit-crash-hidden-only branch July 24, 2025 16:16
stanley31huang pushed a commit that referenced this pull request Aug 14, 2025
…BugFix) (#1996)

* Fix crash in ManifestBrowser when only hidden manifests are present.

* The manifest screen is not shown even in interactive mode if all the manifests are hidde.

* Comply the code with Python 3.5.

* Adjust tests by putting them in a proper test classes + comments. Reduce code duplication.

* Rename ManifestBrowser.get_flattened_values, apply the same fix for _save_manifest in RemoteController, make tests more readable.
bladernr pushed a commit that referenced this pull request Aug 28, 2025
…BugFix) (#1996)

* Fix crash in ManifestBrowser when only hidden manifests are present.

* The manifest screen is not shown even in interactive mode if all the manifests are hidde.

* Comply the code with Python 3.5.

* Adjust tests by putting them in a proper test classes + comments. Reduce code duplication.

* Rename ManifestBrowser.get_flattened_values, apply the same fix for _save_manifest in RemoteController, make tests more readable.
stanley31huang pushed a commit that referenced this pull request Oct 3, 2025
…BugFix) (#1996)

* Fix crash in ManifestBrowser when only hidden manifests are present.

* The manifest screen is not shown even in interactive mode if all the manifests are hidde.

* Comply the code with Python 3.5.

* Adjust tests by putting them in a proper test classes + comments. Reduce code duplication.

* Rename ManifestBrowser.get_flattened_values, apply the same fix for _save_manifest in RemoteController, make tests more readable.
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