Skip to content

Fix UnboundLocalError in _catch_test_status when ThreadPool creation... - #6342

Merged
pevogam merged 1 commit into
avocado-framework:masterfrom
Poppy-mt:master
Aug 27, 2026
Merged

Fix UnboundLocalError in _catch_test_status when ThreadPool creation...#6342
pevogam merged 1 commit into
avocado-framework:masterfrom
Poppy-mt:master

Conversation

@Poppy-mt

@Poppy-mt Poppy-mt commented Aug 26, 2026

Copy link
Copy Markdown

Initialize pool to None and only terminate it if it was created, so a
failure during ThreadPool setup is not masked by UnboundLocalError in
the finally block.

Summary by CodeRabbit

  • Bug Fixes
    • Improved test execution cleanup to prevent errors when the thread pool cannot be created.

@mr-avocado mr-avocado Bot moved this to Review Requested in Default project Aug 26, 2026
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e8e48cf0-a629-4f24-b364-93736c0f2047

📥 Commits

Reviewing files that changed from the base of the PR and between 4e00d7f and 1343a15.

📒 Files selected for processing (1)
  • avocado/core/test.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

_catch_test_status now initializes the multiprocessing pool reference to None before pool creation. Cleanup now terminates the pool only when creation succeeded. This prevents cleanup errors when pool initialization fails.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 1343a

This is a small, localized fix for an exception-handling bug, and no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: preventing an UnboundLocalError in _catch_test_status when ThreadPool creation fails.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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.

@Poppy-mt Poppy-mt changed the title Fix UnboundLocalError in _catch_test_status when ThreadPool creation fails Fix UnboundLocalError in _catch_test_status when ThreadPool creation... Aug 26, 2026
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.61%. Comparing base (ba0306c) to head (f5c128a).
⚠️ Report is 142 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6342      +/-   ##
==========================================
- Coverage   73.66%   70.61%   -3.05%     
==========================================
  Files         206      207       +1     
  Lines       22635    23639    +1004     
==========================================
+ Hits        16673    16692      +19     
- Misses       5962     6947     +985     

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pevogam

pevogam commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Hi @Poppy-mt, I think I can propose a fix for your failing CI variants in #6343. Once it gets merged I will ask you to rebase to the main branch and push again if possible so we can restore the coverage for your PR.

@pevogam

pevogam commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Ok, merged. Could you please rebase on the master branch and push again?

Initialize pool to None and only terminate it if it was created, so a
failure during ThreadPool setup is not masked by UnboundLocalError in
the finally block.

Signed-off-by: mataotao <mataotao@uniontech.com>
@Poppy-mt
Poppy-mt force-pushed the master branch 2 times, most recently from 9d54383 to f5c128a Compare August 27, 2026 01:43
@Poppy-mt

Copy link
Copy Markdown
Author

Ok, merged. Could you please rebase on the master branch and push again?

Hi @pevogam, thanks for merging #6343. I have rebased this PR on the latest master and pushed again.

The previous podman-related CI failures look resolved, but a new error showed up during "Installing Avocado in develop mode" (python3 setup.py develop --user):
error: Could not find suitable distribution for Requirement.parse('idna<4,>=2.5')

This happens before the tests run, while optional plugins are being installed. This PR only changes avocado/core/test.py (initialize pool to None so a ThreadPool setup failure is not masked by UnboundLocalError), so this looks like another environment/dependency issue rather than something introduced by this change.

Could you take a look, or should I just re-run the failed jobs? Thanks.

@pevogam

pevogam commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Hmmm, I wonder if GitHub's severe outage yesterday could have caused some problems in the cache. The tests passed just fine right after yesterday's merge as well.

@Poppy-mt

Copy link
Copy Markdown
Author

Thanks for re-running the jobs. They failed again with the same error.

So it does not look like a one-off cache issue from yesterday's outage. Please let me know if I should wait for a CI-side fix, or if there is anything I should change on this PR. Thanks.

@pevogam

pevogam commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

@PraveenPenguin Do you happen to understand what this avocado cache that fails in the CI is about? It also fails for other PRs too.

@pevogam

pevogam commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Hmmm, I reran the GH Actions workflow from the master branch and all tests pass there. Then reran here too and everything passed. It must have been Github related cache issue.

@pevogam pevogam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@pevogam
pevogam merged commit 400ff59 into avocado-framework:master Aug 27, 2026
175 of 181 checks passed
@github-project-automation github-project-automation Bot moved this from Review Requested to Done 114 in Default project Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done 114

Development

Successfully merging this pull request may close these issues.

2 participants