Skip to content

Conversation

@cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Jul 29, 2022

Background

I'm working towards introducing pants. Eventually I would like to use pants to run tests to take advantage of the fine-grained per-file caching of results that accounts for dependencies by python files (pants infers the deps by reading the python files). But, pants uses pytest to run tests not nosetest.

This is another of several PRs that improves our compatibility with pytest. Others include: #5686, #5689, #5690

Note: I do not have pytest support completely ironed out, but this is a step in that direction. Don't expect running all our tests with pytest to work yet.

Overview

DBTestCase was relying on some unittest/nosetest implmentation details that are not available when running under pytest.

The purpose of those adjustments was: Keep the DB after a test failure (do not drop it) to facilitate debugging.

However, pytest does not make results availble to fixtures. There are some complex workarounds, but there are issues with blindly copying the previous logic. For example, pytest can continue running tests after a failure or error unlike nosetest which we have configured to stop on failures. In order to safely skip dropping the DB, we need to make the DB available in a clean state after the test(s) using it. So, re-implementing this test feature requires something that renames a DB to preserve data for debugging, so that subsequent tests have a clean DB to work with.

I left a TODO note about this for whoever ends up trying to implement this. For now, this just avoids accessing attributes on an TestResult object that pytest does not provide.

@cognifloyd cognifloyd added this to the 3.8.0 milestone Jul 29, 2022
@cognifloyd cognifloyd added no changelog No Changelog.rst needed for this PR refactor tests labels Jul 29, 2022
@cognifloyd cognifloyd self-assigned this Jul 29, 2022
@pull-request-size pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Jul 29, 2022
@cognifloyd cognifloyd requested a review from a team July 29, 2022 01:28
Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

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

LGTM

@cognifloyd cognifloyd enabled auto-merge (squash) August 3, 2022 21:17
@cognifloyd cognifloyd merged commit 2e74993 into master Aug 3, 2022
@cognifloyd cognifloyd deleted the pytest-test-result branch August 3, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog No Changelog.rst needed for this PR pantsbuild refactor size/S PR that changes 10-29 lines. Very easy to review. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants