Skip to content

Implement non-default markers#472

Open
dinhngtu wants to merge 3 commits intomasterfrom
dnt/enable-marker
Open

Implement non-default markers#472
dinhngtu wants to merge 3 commits intomasterfrom
dnt/enable-marker

Conversation

@dinhngtu
Copy link
Copy Markdown
Member

Some markers (e.g. reboot/flaky) represent tests that shouldn't be run in the default suite. As a result, we have to negate them over and over in jobs.py. Add a mechanism to skip them by default. These tests can be manually enabled using a pytest argument.

Add a "slow" marker to represent slow tests for detecting rare issues.

Add a couple initial slow tests to catch previously-encountered issues.

Add some typing to conftest.py while we're at it.

@dinhngtu dinhngtu requested review from a team as code owners April 13, 2026 09:20
@dinhngtu dinhngtu force-pushed the dnt/enable-marker branch from c726566 to 1cef705 Compare April 13, 2026 09:22
@dinhngtu
Copy link
Copy Markdown
Member Author

parametrize made the same test show up 100 times in collect so I scrapped it and went with a normal for loop instead.

@stormi
Copy link
Copy Markdown
Member

stormi commented Apr 13, 2026

parametrize made the same test show up 100 times in collect so I scrapped it and went with a normal for loop instead.

parametrize often looks like a good idea until we actually try to use it :)

Copy link
Copy Markdown
Member

@glehmann glehmann left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, but it is missing some documentation in the README.md.

If we add more excluded makers by default, we might want to add a --enable-all, but I don't think that's necessary for now.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
@dinhngtu dinhngtu force-pushed the dnt/enable-marker branch from 1cef705 to 8557c2f Compare April 14, 2026 10:20
@dinhngtu
Copy link
Copy Markdown
Member Author

Added documentation. Also rearranged pytest.ini to explicitly list the non-default markers.

This avoids us having to specify 'not reboot/flaky' everywhere in
jobs.py by adding an implicit skip marker if the corresponding arguments
are not specified in the pytest call.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
The "slow" marker designates tests that may take a long time and are
designed to catch rare issues that may need large datasets or multiple
repetitions to be reproduced.

Add the following tests to reproduce the following rare issues:
* test_migrate_repeat: Windows migration issues associated with the
  XenVbd driver
* test_create_and_destroy_sr_repeat: NFS deadlock issue

Add a new `multi/slow` VM list for this purpose.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@vates.tech>
@dinhngtu dinhngtu force-pushed the dnt/enable-marker branch from 8557c2f to 77aeed5 Compare April 14, 2026 10:28
Comment thread jobs.py
"paths": ["tests/limits"],
}
},
"slow": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can start with that, but I think we'll quickly see the need for a way to be more specific about which slow tests to run, depending on their requirements.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Indeed I ran into this when trying to add tests/guest_tools/win into the slow suite.

Copy link
Copy Markdown
Member

@stormi stormi left a comment

Choose a reason for hiding this comment

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

I have doubts about some of the choices, but nothing that we can't improve later.

Copy link
Copy Markdown
Member

@glehmann glehmann left a comment

Choose a reason for hiding this comment

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

Please make sure to update vm_data.py in the jenkins-ci/ci-configuration repository before merging this PR

Copy link
Copy Markdown
Member

@stormi stormi left a comment

Choose a reason for hiding this comment

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

On second thought I don't like the implementation. It will cause annoyances when once wants to run a specific test from a failed job, or execute tests manually, only to discover that pytest skipped parts of the tests when it's too late already.

To me, the implementation should be limited to jobs.py: automatically adding "not XXX" to the -m switch, in order to exclude some markers by default, unless they're specified in the job definition or manually via -m on the command line.

@dinhngtu
Copy link
Copy Markdown
Member Author

Using a for loop made per-test setup/teardown more complicated compared to using a fixture, so I'm having second thoughts...

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