Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add subtest_previous PhaseFailureCheckpoint #992

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

gtpalmer
Copy link
Contributor

@gtpalmer gtpalmer commented Nov 29, 2021

The PhaseFailureCheckpoint currently only supports two modes of failure checking: last and all_previous. As noted in the docstring for the PhaseFailureCheckpoint, the all_previous does not play well with subtests:

When using all_previous, this will take in to account all phases; it will
NOT limit itself to the subtest when using the FAIL_SUBTEST action.

To remedy this, this PR adds support for a subtest_previous mode which checks all phases in the same subtest as the checkpoint, or if the checkpoint is not in any subtest falls back to the behavior of all_previous.

This allows one to use a checkpoint to short-circuit execution of a subtest, without quitting the overall test. A good example of this can be seen in the unittest test_subtest_previous_fail_subtest__fail_in_subtest. We construct a phasegroup as follows:

htf.Subtest(
         htf.Test(
        phase0,
        htf.Subtest(
          'sub', fail_phase, phase1, 
          phase_branches.PhaseFailureCheckpoint.subtest_previous(
            'subtest_previous_fail_subtest_in_subtest', action=htf.PhaseResult.FAIL_SUBTEST), 
          skip0,
        ),
        phase2)

Which results in the following execution:

  • phase0: PASS
  • fail_phase: FAIL
  • phase1: PASS
  • checkpoint: FAIL_SUBTEST
  • skip0: SKIP
  • phase2: PASS

This change is Reviewable

@gtpalmer gtpalmer force-pushed the subtest_previous_checkpoint branch from bb914e2 to 2a89e57 Compare November 29, 2021 16:01
elif self.previous_phases_to_check == PreviousPhases.SUBTEST and \
subtest_rec is not None:
for phase_rec in phase_records:
if phase_rec.subtest_name == subtest_rec.name and \
Copy link
Collaborator

Choose a reason for hiding this comment

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

We prefer not using backslash for line continuation, please use braces instead there and at other lines were it is used.

Copy link
Contributor Author

@gtpalmer gtpalmer Sep 9, 2022

Choose a reason for hiding this comment

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

Updated to wrap in parentheses.

Apologies for already squashing - updated lines are here.

@gtpalmer gtpalmer force-pushed the subtest_previous_checkpoint branch from 2a89e57 to 5e67e0a Compare September 9, 2022 10:33
@cricdecyan
Copy link
Collaborator

cricdecyan commented Sep 15, 2022

@gtpalmer Please update your PR to the latest so I can review it. Thanks.

@gtpalmer gtpalmer force-pushed the subtest_previous_checkpoint branch from 5e67e0a to 6f04d12 Compare September 17, 2022 21:01
@gtpalmer
Copy link
Contributor Author

@cricdecyan Should be once again up to date with master. Looking forward to your feedback.

@cricdecyan cricdecyan force-pushed the subtest_previous_checkpoint branch from 6f04d12 to 4f3791b Compare September 19, 2022 18:49
@cricdecyan cricdecyan merged commit c34e3cc into google:master Sep 19, 2022
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