Skip to content

fix: set ok=False when TestAbortAll is raised in run_suite_class#1006

Open
devimano2011 wants to merge 1 commit intogoogle:masterfrom
devimano2011:fix/set-ok-false-on-test-abort-all
Open

fix: set ok=False when TestAbortAll is raised in run_suite_class#1006
devimano2011 wants to merge 1 commit intogoogle:masterfrom
devimano2011:fix/set-ok-false-on-test-abort-all

Conversation

@devimano2011
Copy link
Copy Markdown
Contributor

Related to #950 and #960

Problem

In run_suite_class() in suite_runner.py, when signals.TestAbortAll is raised, the except block only has pass and does not set ok = False. This means if a test aborts before any failures, the suite exits with code 0 instead of 1.

This is the same issue that was fixed in test_runner.py by PR #997 for issue #960, but run_suite_class() in suite_runner.py was missed.

Fix

Change pass to ok = False in the except signals.TestAbortAll block so that aborting a test suite always results in a non-zero exit code.

Set ok to False when TestAbortAll is caught to indicate failure.
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 30, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@xpconanfan
Copy link
Copy Markdown
Collaborator

In our usage, abort is usually intentional, so we don't necessarily consider this "not ok".
Most common case is, a device doesn't support the feature a test is meant to test, hence abort.

So I'm not sure whether this should be considered "not ok".

@devimano2011
Copy link
Copy Markdown
Contributor Author

Thank you for the feedback! That's a valid point — I wasn't aware that abort is often intentional in your usage (e.g. device doesn't support the feature under test).

Looking at PR #997 more carefully, that fix was motivated by cases where TestAbortAll is raised due to an unexpected failure, where exiting with code 0 would silently hide the problem from CI.

Given your clarification, I see a few options:

  1. Revert this PR — keep pass in run_suite_class since aborting is considered intentional there.
    1. Make it configurable — distinguish intentional aborts (feature not supported) from unexpected ones.
    1. Keep the fix — if consistency with test_runner.py is preferred regardless.
      I'll defer to your judgment. Happy to close/revert if this doesn't align with the intended behavior.

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