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

Skip test scripts in PR testing using pytest markers. #15872

Conversation

yutongzhang-microsoft
Copy link
Contributor

Description of PR

Previously, we used the .azure-pipelines/pr_test_skip_scripts.yaml file to manually specify test scripts to skip during PR testing. However, in our new PR testing model proposed in #15666, test scripts are collected automatically, eliminating the need for this hardcoded file.

To handle skipped scripts in the new model, we now identify them directly within the test scripts. Specifically, we use the pytest.mark.device_type('physical') marker to indicate scripts that can only run on physical testbeds and should be skipped in PR testing. This PR adds the necessary markers to the relevant scripts to align with the new approach.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

Previously, we used the .azure-pipelines/pr_test_skip_scripts.yaml file to manually specify test scripts to skip during PR testing. However, in our new PR testing model proposed in #15666, test scripts are collected automatically, eliminating the need for this hardcoded file.

To handle skipped scripts in the new model, we now identify them directly within the test scripts. Specifically, we use the pytest.mark.device_type('physical') marker to indicate scripts that can only run on physical testbeds and should be skipped in PR testing. This PR adds the necessary markers to the relevant scripts to align with the new approach.

How did you do it?

This PR adds the necessary markers to the relevant scripts to align with the new approach.

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Copy link
Contributor

@xwjiang-ms xwjiang-ms left a comment

Choose a reason for hiding this comment

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

LGTM

@wangxin
Copy link
Collaborator

wangxin commented Dec 5, 2024

With this new mark, it would be easily for people to consider these scripts are physical testbed only.
However, we would like to cover them on VS testbed during PR testing. While running them on VS testbed, we can skip traffic testing and only cover configurations, and probably also cover ASIC DB validation.

Considering that, hard code a mark to indicate that these scripts are physical testbed only does not really make sense to me.

@yutongzhang-microsoft
Copy link
Contributor Author

this new mark, it would be easily for people to consider these scripts are physical

Based on prior analysis, these scripts were confirmed to run exclusively on physical testbeds and were therefore added to the skip list, excluding them from PR testing. For scripts partially compatible with PR testing, they are not in the skip list. So the scripts in skip list can be considered as 'physical testbed only'.

@yutongzhang-microsoft yutongzhang-microsoft deleted the yutongzhang/add_physical_mark branch December 24, 2024 02:00
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