Make E2E tests more comprehensive and less flaky

requirements.txt

@@ -1,5 +1,6 @@
 click
 pytest
+pytest-order
 mypy
 pyinstaller
 requests

tests/e2e/commands/test_check.py

@@ -2,14 +2,14 @@
 
 
 def test_check_git(runner: BinaryRunner) -> None:
-    """Test the check git command output."""
+    """check git command output indicates git is installed and configured."""
     res = runner.run(["check", "git"])
     res.assert_success()
-    res.assert_stdout_contains("Git is installed")
+    res.assert_stdout_contains("Git is installed and configured")
 
 
 def test_check_github(runner: BinaryRunner) -> None:
-    """Test the check gh command output."""
+    """check github command output indicates Github CLI is installed and configured."""
     res = runner.run(["check", "github"])
     res.assert_success()
-    res.assert_stdout_contains("Github CLI is installed")
+    res.assert_stdout_contains("Github CLI is installed and configured")

tests/e2e/commands/test_download.py

@@ -1,28 +1,20 @@
+import json
 from pathlib import Path
 
-from ..constants import EXERCISE_NAME, HANDS_ON_NAME
-from ..runner import BinaryRunner
+from ..constants import EXERCISE_NAME
 
 
-def test_download_exercise(runner: BinaryRunner, exercises_dir: Path) -> None:
-    """Test the download command output successfully performs the download for exercise."""
-    res = runner.run(["download", EXERCISE_NAME], cwd=exercises_dir)
-    res.assert_success()
+def test_download_exercise(downloaded_exercise_dir: Path) -> None:
+    """download creates the exercise folder with its config and README."""
+    assert downloaded_exercise_dir.is_dir()
 
-    exercise_folder = exercises_dir / EXERCISE_NAME
-    assert exercise_folder.is_dir()
-
-    exercise_config = exercise_folder / ".gitmastery-exercise.json"
+    exercise_config = downloaded_exercise_dir / ".gitmastery-exercise.json"
     assert exercise_config.is_file()
+    assert json.loads(exercise_config.read_text())["exercise_name"] == EXERCISE_NAME
 
-    exercise_readme = exercise_folder / "README.md"
-    assert exercise_readme.is_file()
-
+    assert (downloaded_exercise_dir / "README.md").is_file()
 
-def test_download_hands_on(runner: BinaryRunner, exercises_dir: Path) -> None:
-    """Test the download command output successfully performs the download for hands-on."""
-    res = runner.run(["download", HANDS_ON_NAME], cwd=exercises_dir)
-    res.assert_success()
 
-    hands_on_folder = exercises_dir / HANDS_ON_NAME
-    assert hands_on_folder.is_dir()
+def test_download_hands_on(downloaded_hands_on_dir: Path) -> None:
+    """download creates the hands-on folder."""
+    assert downloaded_hands_on_dir.is_dir()

tests/e2e/commands/test_progress.py

@@ -1,36 +1,38 @@
+import json
 from pathlib import Path
+import pytest
 
-from ..constants import EXERCISE_NAME
 from ..runner import BinaryRunner
 
 
-def test_progress_show(runner: BinaryRunner, exercises_dir: Path) -> None:
-    """Test that progress show displays progress."""
-    res = runner.run(["progress", "show"], cwd=exercises_dir)
+def test_progress_show(runner: BinaryRunner, gitmastery_root: Path) -> None:
+    """progress show displays the progress header."""
+    res = runner.run(["progress", "show"], cwd=gitmastery_root)
     res.assert_success()
     res.assert_stdout_contains("Your Git-Mastery progress:")
 
 
-def test_progress_sync_on_then_off(runner: BinaryRunner, exercises_dir: Path) -> None:
-    """Test that progress sync on followed by sync off works correctly."""
-    # Enable sync
-    res_on = runner.run(["progress", "sync", "on"], cwd=exercises_dir)
+def test_progress_sync_on_then_off(runner: BinaryRunner, gitmastery_root: Path) -> None:
+    """progress sync on/off toggles progress_remote in the config."""
+    res_on = runner.run(["progress", "sync", "on"], cwd=gitmastery_root)
     res_on.assert_success()
-    res_on.assert_stdout_contains(
-        "You have setup the progress tracker for Git-Mastery!"
-    )
+    res_on.assert_stdout_contains("You have setup the progress tracker for Git-Mastery!")
+    assert json.loads((gitmastery_root / ".gitmastery.json").read_text())["progress_remote"] is True
 
-    # Disable sync (send 'y' to confirm)
+    # send 'y' to confirm
     res_off = runner.run(
-        ["progress", "sync", "off"], cwd=exercises_dir, stdin_text="y\n"
+        ["progress", "sync", "off"], cwd=gitmastery_root, stdin_text="y\n"
     )
     res_off.assert_success()
     res_off.assert_stdout_contains("Successfully removed your remote sync")
+    assert json.loads((gitmastery_root / ".gitmastery.json").read_text())["progress_remote"] is False
 
 
-def test_progress_reset(runner: BinaryRunner, exercises_dir: Path) -> None:
-    """Test that progress reset works correctly after verify has run."""
-    exercise_dir = exercises_dir / EXERCISE_NAME
-    res = runner.run(["progress", "reset"], cwd=exercise_dir)
-    # TODO: verify that the progress has actually been reset
+@pytest.mark.order(after="tests/e2e/commands/test_verify.py::test_verify_exercise")
+def test_progress_reset(runner: BinaryRunner, verified_exercise_dir: Path) -> None:
+    """progress reset removes the current exercise's entry from progress.json."""
+    res = runner.run(["progress", "reset"], cwd=verified_exercise_dir)
     res.assert_success()
+    progress_json = verified_exercise_dir.parent / "progress" / "progress.json"
+    # TODO: need to verify that the exercise itself progress was reset, not just progress.json was cleared
+    assert json.loads(progress_json.read_text()) == []

tests/e2e/commands/test_setup.py

@@ -1,19 +1,20 @@
+import json
 from pathlib import Path
 
 
-def test_setup(exercises_dir: Path) -> None:
-    """
-    Test that setup creates the progress directory, progress.json, .gitmastery.json and .gitmastery.log
-    Setup command already called in conftest.py for test setup
-    """
-    progress_dir = exercises_dir / "progress"
-    assert progress_dir.is_dir(), f"Expected {progress_dir} to exist"
+def test_setup(setup_gitmastery_root: Path) -> None:
+    """setup creates the expected directory structure, config, and empty progress file."""
+    progress_dir = setup_gitmastery_root / "progress"
+    assert progress_dir.is_dir()
 
     progress_file = progress_dir / "progress.json"
-    assert progress_file.is_file(), f"Expected {progress_file} to exist"
+    assert progress_file.is_file()
+    assert json.loads(progress_file.read_text()) == []
 
-    config_file = exercises_dir / ".gitmastery.json"
-    assert config_file.is_file(), f"Expected {config_file} to exist"
+    config_file = setup_gitmastery_root / ".gitmastery.json"
+    assert config_file.is_file()
+    config = json.loads(config_file.read_text())
+    assert config["progress_local"] is True
+    assert config["progress_remote"] is False
 
-    log_file = exercises_dir / ".gitmastery.log"
-    assert log_file.is_file(), f"Expected {log_file} to exist"
+    assert (setup_gitmastery_root / ".gitmastery.log").is_file()

tests/e2e/commands/test_verify.py

@@ -1,14 +1,16 @@
+import json
 from pathlib import Path
 
 from ..constants import EXERCISE_NAME
-from ..runner import BinaryRunner
 
 
-def test_verify_exercise(runner: BinaryRunner, exercises_dir: Path) -> None:
-    """Test that verify runs on a downloaded exercise."""
-    exercise_dir = exercises_dir / EXERCISE_NAME
-    res = runner.run(["verify"], cwd=exercise_dir)
-    res.assert_success()
-    # TODO: check that the correct tests have been run
-    res.assert_stdout_contains("Starting verification of")
-    res.assert_stdout_contains("Verification completed.")
+def test_verify_exercise(verified_exercise_dir: Path) -> None:
+    """verify writes a progress entry with the expected fields."""
+    progress_json = verified_exercise_dir.parent / "progress" / "progress.json"
+    entries = json.loads(progress_json.read_text())
+    assert len(entries) == 1
+    entry = entries[0]
+    assert entry["exercise_name"] == EXERCISE_NAME
+    assert "started_at" in entry
+    assert "completed_at" in entry
+    assert "status" in entry

tests/e2e/conftest.py

@@ -3,41 +3,82 @@
 
 import pytest
 
+from .constants import EXERCISE_NAME, HANDS_ON_NAME
 from .utils import rmtree
 from .runner import BinaryRunner
 
 
 @pytest.fixture(scope="session")
 def runner() -> BinaryRunner:
     """
-    Return a BinaryRunner instance for the gitmastery binary.
+    BinaryRunner pointed at the gitmastery binary.
     """
     return BinaryRunner.from_env()
 
 
-@pytest.fixture(scope="session")
-def exercises_dir(
+def _make_gitmastery_root(
     runner: BinaryRunner, tmp_path_factory: pytest.TempPathFactory
 ) -> Generator[Path, None, None]:
-    """
-    Run setup once and return the path to the exercises directory.
-    Tears down by deleting the entire working directory after all tests complete.
-    """
-    work_dir = tmp_path_factory.mktemp("e2e-tests-tmp")
+    work_dir = tmp_path_factory.mktemp("gitmastery-e2e-test-tmp")
 
-    # Send newline to accept the default directory name prompt
     res = runner.run(["setup"], cwd=work_dir, stdin_text="\n")
-    assert res.returncode == 0, (
-        f"Setup failed with exit code {res.returncode}\n"
-        f"stdout:\n{res.stdout}\nstderr:\n{res.stderr}"
-    )
+    res.assert_success()
 
-    exercises_path = work_dir / "gitmastery-exercises"
-    assert exercises_path.is_dir(), (
-        f"Expected directory {exercises_path} to exist after setup"
-    )
+    gitmastery_root_folder = work_dir / "gitmastery-exercises"
+    assert gitmastery_root_folder.is_dir()
 
     try:
-        yield exercises_path
+        yield gitmastery_root_folder
     finally:
-        rmtree(work_dir)  # ensure cleanup even if tests fail
+        rmtree(work_dir)
+
+
+@pytest.fixture(scope="session")
+def gitmastery_root(
+    runner: BinaryRunner, tmp_path_factory: pytest.TempPathFactory
+) -> Generator[Path, None, None]:
+    """
+    Run `setup` once per session, yield the exercises path.
+    """
+    yield from _make_gitmastery_root(runner, tmp_path_factory)
+
+
+@pytest.fixture
+def setup_gitmastery_root(
+    runner: BinaryRunner, tmp_path_factory: pytest.TempPathFactory
+) -> Generator[Path, None, None]:
+    """
+    For use by test_setup only, which must inspect pristine initial state.
+    """
+    yield from _make_gitmastery_root(runner, tmp_path_factory)
+
+
+@pytest.fixture(scope="session")
+def downloaded_exercise_dir(runner: BinaryRunner, gitmastery_root: Path) -> Path:
+    """
+    Run `download EXERCISE_NAME` once per session, return the exercise dir.
+    """
+    res = runner.run(["download", EXERCISE_NAME], cwd=gitmastery_root)
+    res.assert_success()
+    exercise_dir = gitmastery_root / EXERCISE_NAME
+    return exercise_dir
+
+
+@pytest.fixture(scope="session")
+def downloaded_hands_on_dir(runner: BinaryRunner, gitmastery_root: Path) -> Path:
+    """
+    Run `download HANDS_ON_NAME` once per session, return the hands-on dir.
+    """
+    res = runner.run(["download", HANDS_ON_NAME], cwd=gitmastery_root)
+    res.assert_success()
+    return gitmastery_root / HANDS_ON_NAME
+
+
+@pytest.fixture(scope="session")
+def verified_exercise_dir(runner: BinaryRunner, downloaded_exercise_dir: Path) -> Path:
+    """
+    Run `verify` on the downloaded exercise once per session, return the exercise dir.
+    """
+    res = runner.run(["verify"], cwd=downloaded_exercise_dir)
+    res.assert_success()
+    return downloaded_exercise_dir