Skip to content

Conversation

@codambro
Copy link
Contributor

@codambro codambro commented Oct 15, 2025

Handle subtest names cleanly when defining test description, to avoid duplicate test cases in generated TAP file.

To accept your contribution, please complete the checklist below.

  • Is your name/identity in the AUTHORS file alphabetically?
  • Did you write a test to verify your code change?
  • Is CI passing? Coverage has been failing since September, unrelated to this Pull Request

Testing

Sample pytest code:

def test_subtests(subtests):
    """Simple subtest example"""
    for i in range(2):
        with subtests.test(msg="is_even", i=i):
            assert i % 2 == 0, f"{i} is odd"

Before

1..1
# TAP results for samples/test_sample.py
ok 1 samples/test_sample.py::test_subtests
not ok 2 samples/test_sample.py::test_subtests
ok 3 samples/test_sample.py::test_subtests

Note how pytest-tap still itemizes every subtest individually. But the descriptions are identical between them. Additionally, the Plan line states "1..1" despite lines indicating 1..3

After

1..3
# TAP results for samples/test_sample.py
ok 1 samples/test_sample.py::test_subtests[is_even] (i=0)
not ok 2 samples/test_sample.py::test_subtests[is_even] (i=1)
ok 3 samples/test_sample.py::test_subtests

Cody D'Ambrosio and others added 6 commits October 15, 2025 09:48
Handle subtest names cleanly when defining test description, to avoid duplicate test cases in generated TAP file.
Update plan reporting so all subtests get included.
@mblayman
Copy link
Member

I'm open to this addition, but I think there need to be some structural test changes first. What I would like to avoid is this package accidentally depending on pytest-subtests in hidden ways even though it is not a required dependency. Thus, I think that we should not modify the default test execution to include pytest-subtests. Instead, an additional tox execution should be added to run the test suite when pytest-subtests is present. That may require some marks to split behavior for conditional running certain unit tests, but it will give clean separation between the base mode and the mode where subtests is present.

I may have time to look into the separate coverage issue to see what this failing.

@mblayman
Copy link
Member

I have fixed the test coverage in main. You can merge in main and should be able to get that fix.

@codambro
Copy link
Contributor Author

Per your request, I created separate tox environments for subtest related testing.

Copy link
Member

@mblayman mblayman left a comment

Choose a reason for hiding this comment

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

From the behavior I can see in the test expectations, this looks pretty good functionally. I want to understand the behavior a bit better before we add some of these conditionals to see if there is a cleaner way to handle this behavior.

if option.tap_stream or option.tap_combined:
if (option.tap_stream or option.tap_combined) and not (
session.config.pluginmanager.has_plugin("subtests")
):
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this logic. Can you help me understand why we don't want to write the plan when the subtests plugin is available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pytest does not know about subtests at collection time. So if I have 1 test with 3 subtests, the test plan will show "1..1". But then the TAP file will contain 4 lines.

def pytest_unconfigure(self, config: pytest.Config):
"""Dump the results."""
if self._tracker.combined and config.pluginmanager.has_plugin("subtests"):
# Force Tracker to write plan at beginning of file
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we didn't write the test plan earlier, tap-py will use the number of lines to determine the test plan (combined_line_number). From my example above, that will correctly be "1..4". However, tap-py writes the test plan at the end of the file. Not sure why it was implemented that way, though it's still valid TAP formatting.

To try to be as consistent as possible when subtests are vs are not used, I wanted the resulting TAP file to look similar. So by calling "set_plan" before generating the report, it ensures that "1..4" appears at the top of the file instead.

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