Quarantined tests still failing are removed on next run

[js-dieu]

Hello

Situation
Consider the following test module:

def test_will_fail():
    assert False

def test_will_succeed():
    assert True

Given the following quarantine.txt file:
test_that.py::test_will_fail
Running the test suite with the following command line
pytest --quarantine quarantine.txt --save-quarantine quarantine.txt
causes the quarantine file to be empty.

Expectation
I would have expected quarantined tests still failing not to be removed from the newly generated quarantine.

Proposal
Do not remove tests already quarantined from the quarantine file if they fail to complete.
If this behavior is to be kept, propose an option for doing so.

Side note
I can propose a PR if you want.

Environment

• python: 3.6.8
• pytest: 6.0.1
• pytest-quarantine: 2.0.0

[bhrutledge]

Thanks for writing up a detailed issue! Also, I'm happy to see that you're using this with pytest 6; I haven't tested that yet. 😉

Running the test suite with the following command line
pytest --quarantine quarantine.txt --save-quarantine quarantine.txt

Can you describe your use case here? In my use thus far, I've used either the --quarantine or --save-quarantine, but not both at the same time. I can imagine the behavior when using both would be tricky to get right, so I'm more tempted to raise an error if they're used together.

[js-dieu]

It seems it runs fine with pytest 6 ;)

I have a long running suite of test (approx. 2h). Using both options at the same time is intended to save time. I cannot afford running this suite twice.

I do not think that raising an error is a good solution as long as it would require two runs:

• one for generating the quarantine
• the other for running with the quarantine.

Moreover this would clearly lack dynamism by adding extra burden on the CI unnecessarily. I believe we can achieve using both options at the same time.

[bhrutledge]

Thanks for describing your situation. Can you share more details about your CI configuration (and/or a link to the repo)? Are you always running your test suite with both options? If so, that was not the intended use (which is not to say we wouldn't support it); the --save-quarantine option is intended to be used occasionally, when there's been a substantial change in the expected failures. If you're not using both options all the time, what determines when you use them?

Also, as an alternative to --save-quarantine, it's perfectly fine to manually add or remove lines from quarantine.txt.

[js-dieu]

Ok, I do not get the point regarding your plugin's usage. I now better understand why there are two plugin classes ;)
Unfortunately I cannot share the repository link as it is hosted on a private instance unreachable from the internet.

To give you additional insights, we have a tests suite containing thousands of tests, all of them taking 2h to run. The integration process involves tens of developers and given the amount of code we have a two step acceptance workflow for incoming development.
When a tests fail, we skip it until a fix is written (usually a 2-3 hours story) in order not to block the CI. Skipping a test is just a nonsense (and is hard to follow), I prefer marking it as being in quarantine so that it does not impact the overall status anymore but kept being run. Meanwhile, It allows me to easily notify the person in charge of the test/development in order to get a fix and appropriately define a workflow for test deprecation. Even more it allows me to get a quality metric regarding the integration.

The idea was to use your plugin to maintain a list of quarantined tests in case of failure in our CI. But given the amount of time for running the test suite, I would appreciate to get a way to use both options at the same time.

[bhrutledge]

Thanks for the details. I'll admit that I still don't quite follow (for example, it seems possible to miss a new failure if it's automatically quarantined). I do think it's worth explicitly defining the behavior of using both options, I'm just not sure what that should be yet. I was hoping to experiment with it, but other things have taken precedence.

So, if you're game to submit a PR, I'm game to review it, with the caveat that there might be unexpected side-effects that prevent supporting your desired behavior.