Require quarantine path on command line (#30)

* Update README

* Require PATH argument

* Simplify tests

* Update CHANGELOG

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased]
 
+### Fixed
+
+- --save-quarantine can overwrite test files ([#29](https://github.com/EnergySage/pytest-quarantine/issues/29))
+
 ## [1.2.0] - 2019-11-13
 
 ### Changed

README.md

@@ -36,7 +36,7 @@ $ pip install pytest-quarantine
 Run your test suite and save the failing tests to `quarantine.txt`:
 
 ```
-$ pytest --save-quarantine
+$ pytest --save-quarantine=quarantine.txt
 ============================= test session starts ==============================
 ...
 collected 1380 items
@@ -52,7 +52,7 @@ Add `quarantine.txt` to your version control system.
 Run your test suite with the quarantined tests marked as expected failures:
 
 ```
-$ pytest --quarantine
+$ pytest --quarantine=quarantine.txt
 ============================= test session starts ==============================
 ...
 collected 1380 items
@@ -65,14 +65,6 @@ added mark.xfail to 661 of 661 items from quarantine.txt
 
 When the expected failures eventually pass (i.e., they get counted as `xpassed`), they can be removed manually from `quarantine.txt`, or automatically using `--save-quarantine`. Note that the latter will overwrite the contents of the quarantine, so it's best to only use it when running the entire test suite.
 
-The default `quarantine.txt` can be changed by an optional argument (for example, if test failures differ between environments, or for multiple test suites):
-
-```
-$ pytest --save-quarantine=quarantine-py3.txt
-
-$ pytest --quarantine=quarantine-py3.txt
-```
-
 ## Getting help
 
 Please submit questions, bug reports, and feature requests in the [issue tracker](https://github.com/EnergySage/pytest-quarantine/issues).

src/pytest_quarantine.py

@@ -15,11 +15,6 @@
 import pytest
 
 
-# TODO: Guarantee this is opened from pytest's root dir
-# (to allow running pytest in a subdirectory)
-DEFAULT_QUARANTINE = "quarantine.txt"
-
-
 def _item_count(nodeids):
     count = len(nodeids)
     return "{} item{}".format(count, "" if count == 1 else "s")
@@ -124,16 +119,12 @@ def pytest_addoption(parser):
 
     group.addoption(
         "--save-quarantine",
-        nargs="?",
-        const=DEFAULT_QUARANTINE,
         metavar="PATH",
-        help="Write failing tests to %(metavar)s (default: %(const)s)",
+        help="Write failing test ID's to %(metavar)s",
     )
 
     group.addoption(
         "--quarantine",
-        nargs="?",
-        const=DEFAULT_QUARANTINE,
         metavar="PATH",
-        help="Mark tests listed in %(metavar)s with `xfail` (default: %(const)s)",
+        help="Mark test ID's listed in %(metavar)s with `xfail`",
     )

tests/test_quarantine.py

@@ -15,21 +15,25 @@
     # Python 2), use the private constants for readability.
     from _pytest.main import EXIT_OK, EXIT_TESTSFAILED, EXIT_USAGEERROR  # noqa: F401
 
-from pytest_quarantine import DEFAULT_QUARANTINE
+QUARANTINE_PATH = "quarantine.txt"
 
 
-def test_default_file():
-    assert DEFAULT_QUARANTINE == "quarantine.txt"
-
-
-def test_options(testdir):
+def test_options_in_help(testdir):
     result = testdir.runpytest("--help")
 
     result.stdout.fnmatch_lines(
-        ["quarantine:", "*--save-quarantine=*", "*--quarantine=*"]
+        ["quarantine:", "*--save-quarantine=PATH*", "*--quarantine=PATH*"]
     )
 
 
+@pytest.mark.parametrize("option", ["--quarantine", "--save-quarantine"])
+def test_options_require_path(option, testdir):
+    result = testdir.runpytest(option)
+
+    assert result.ret == EXIT_USAGEERROR
+    result.stderr.fnmatch_lines(["*error:*expected one argument"])
+
+
 @pytest.fixture
 def error_failed_passed(testdir):
     """Create test_error_failed_passed.py with one test for each outcome."""
@@ -53,9 +57,11 @@ def test_passed():
     )
 
 
-def test_no_output_without_options(testdir):
+def test_no_output_without_options(testdir, error_failed_passed):
     result = testdir.runpytest()
-    assert DEFAULT_QUARANTINE not in result.stdout.str()
+
+    assert result.ret == EXIT_TESTSFAILED
+    assert QUARANTINE_PATH not in result.stdout.str()
 
 
 @pytest.fixture
@@ -75,22 +81,17 @@ def _write_path(path, content):
     return testdir
 
 
-@pytest.mark.parametrize("quarantine_path", [DEFAULT_QUARANTINE, ".quarantine"])
-def test_save_failing_tests(quarantine_path, testdir, error_failed_passed):
-    args = ["--save-quarantine"]
-    if quarantine_path != DEFAULT_QUARANTINE:
-        args.append(quarantine_path)
-
-    result = testdir.runpytest(*args)
+def test_save_failing_tests(testdir, error_failed_passed):
+    result = testdir.runpytest("--save-quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_TESTSFAILED
+    result.assert_outcomes(passed=1, failed=1, error=1)
     result.stdout.fnmatch_lines(
-        ["*- 2 items saved to {} -*".format(quarantine_path), "=*failed*"]
+        ["*- 2 items saved to {} -*".format(QUARANTINE_PATH), "=*failed*"]
     )
-    result.assert_outcomes(passed=1, failed=1, error=1)
-    assert result.ret == EXIT_TESTSFAILED
 
     assert testdir.path_has_content(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_error_failed_passed.py::test_error
         test_error_failed_passed.py::test_failed
@@ -99,9 +100,6 @@ def test_save_failing_tests(quarantine_path, testdir, error_failed_passed):
 
 
 def test_dont_save_other_outcomes(testdir):
-    quarantine_path = DEFAULT_QUARANTINE
-    args = ["--save-quarantine"]
-
     testdir.makepyfile(
         """\
         import pytest
@@ -123,21 +121,18 @@ def test_xpassed():
         """
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("--save-quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_OK
+    result.assert_outcomes(passed=1, skipped=1, xpassed=1, xfailed=1)
     result.stdout.fnmatch_lines(
-        ["*- 0 items saved to {} -*".format(quarantine_path), "=*skipped*"]
+        ["*- 0 items saved to {} -*".format(QUARANTINE_PATH), "=*skipped*"]
     )
-    result.assert_outcomes(passed=1, skipped=1, xpassed=1, xfailed=1)
-    assert result.ret == EXIT_OK
 
-    assert testdir.path_has_content(quarantine_path, "")
+    assert testdir.path_has_content(QUARANTINE_PATH, "")
 
 
 def test_save_empty_quarantine(testdir):
-    quarantine_path = DEFAULT_QUARANTINE
-    args = ["--save-quarantine"]
-
     testdir.makepyfile(
         test_xpassed="""\
         def test_passed():
@@ -146,146 +141,124 @@ def test_passed():
     )
 
     testdir.write_path(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_xpassed.py::test_passed
         """,
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("--save-quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_OK
+    result.assert_outcomes(passed=1)
     result.stdout.fnmatch_lines(
-        ["*- 0 items saved to {} -*".format(quarantine_path), "=*passed*"]
+        ["*- 0 items saved to {} -*".format(QUARANTINE_PATH), "=*passed*"]
     )
-    result.assert_outcomes(passed=1)
-    assert result.ret == EXIT_OK
-
-    assert testdir.path_has_content(quarantine_path, "")
 
+    assert testdir.path_has_content(QUARANTINE_PATH, "")
 
-@pytest.mark.parametrize("quarantine_path", [DEFAULT_QUARANTINE, ".quarantine"])
-def test_missing_quarantine(quarantine_path, testdir):
-    args = ["--quarantine"]
-    if quarantine_path != DEFAULT_QUARANTINE:
-        args.append(quarantine_path)
 
-    result = testdir.runpytest(*args)
+def test_missing_quarantine(testdir):
+    result = testdir.runpytest("--quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_USAGEERROR
     result.stderr.fnmatch_lines(
-        ["ERROR: Could not load quarantine:*'{}'".format(quarantine_path)]
+        ["ERROR: Could not load quarantine:*'{}'".format(QUARANTINE_PATH)]
     )
-    assert result.ret == EXIT_USAGEERROR
-
 
-@pytest.mark.parametrize("quarantine_path", [DEFAULT_QUARANTINE, ".quarantine"])
-def test_full_quarantine(quarantine_path, testdir, error_failed_passed):
-    args = ["--quarantine"]
-    if quarantine_path != DEFAULT_QUARANTINE:
-        args.append(quarantine_path)
 
+def test_full_quarantine(testdir, error_failed_passed):
     testdir.write_path(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_error_failed_passed.py::test_error
         test_error_failed_passed.py::test_failed
         """,
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("--quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_OK
+    result.assert_outcomes(passed=1, xfailed=2)
     result.stdout.fnmatch_lines(
         [
             "collected*",
-            "added mark.xfail to 2 of 2 items from {}".format(quarantine_path),
+            "added mark.xfail to 2 of 2 items from {}".format(QUARANTINE_PATH),
         ]
     )
-    result.assert_outcomes(passed=1, xfailed=2)
-    assert result.ret == EXIT_OK
 
 
 def test_partial_quarantine(testdir, error_failed_passed):
-    quarantine_path = DEFAULT_QUARANTINE
-    args = ["--quarantine"]
-
     testdir.write_path(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_error_failed_passed.py::test_failed
         test_error_failed_passed.py::test_extra
         """,
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("--quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_TESTSFAILED
+    result.assert_outcomes(passed=1, error=1, xfailed=1)
     result.stdout.fnmatch_lines(
         [
             "collected*",
-            "added mark.xfail to 1 of 2 items from {}".format(quarantine_path),
+            "added mark.xfail to 1 of 2 items from {}".format(QUARANTINE_PATH),
         ]
     )
-    result.assert_outcomes(passed=1, error=1, xfailed=1)
-    assert result.ret == EXIT_TESTSFAILED
 
 
 def test_only_extra_quarantine(testdir, error_failed_passed):
-    quarantine_path = DEFAULT_QUARANTINE
-    args = ["--quarantine"]
-
     testdir.write_path(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_error_failed_passed.py::test_extra
         """,
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("--quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_TESTSFAILED
+    result.assert_outcomes(passed=1, failed=1, error=1)
     result.stdout.fnmatch_lines(
         [
             "collected*",
-            "added mark.xfail to 0 of 1 item from {}".format(quarantine_path),
+            "added mark.xfail to 0 of 1 item from {}".format(QUARANTINE_PATH),
         ]
     )
-    result.assert_outcomes(passed=1, failed=1, error=1)
-    assert result.ret == EXIT_TESTSFAILED
 
 
 def test_passing_quarantine(testdir, error_failed_passed):
-    quarantine_path = DEFAULT_QUARANTINE
-    args = ["--quarantine"]
-
     testdir.write_path(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_error_failed_passed.py::test_error
         test_error_failed_passed.py::test_failed
         test_error_failed_passed.py::test_passed
         """,
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("--quarantine", QUARANTINE_PATH)
 
+    assert result.ret == EXIT_OK
+    result.assert_outcomes(xfailed=2, xpassed=1)
     result.stdout.fnmatch_lines(
         [
             "collected*",
-            "added mark.xfail to 3 of 3 items from {}".format(quarantine_path),
+            "added mark.xfail to 3 of 3 items from {}".format(QUARANTINE_PATH),
         ]
     )
-    result.assert_outcomes(xfailed=2, xpassed=1)
-    assert result.ret == EXIT_OK
 
 
 def test_no_report_with_quiet_option(testdir, error_failed_passed):
-    quarantine_path = DEFAULT_QUARANTINE
-    args = ["-q", "--quarantine"]
-
     testdir.write_path(
-        quarantine_path,
+        QUARANTINE_PATH,
         """\
         test_error_failed_passed.py::test_error
         """,
     )
 
-    result = testdir.runpytest(*args)
+    result = testdir.runpytest("-q", "--quarantine", QUARANTINE_PATH)
 
-    assert quarantine_path not in result.stdout.str()
+    assert QUARANTINE_PATH not in result.stdout.str()