From b68300ba00dff0acada1fdbbb32e71f23451e56d Mon Sep 17 00:00:00 2001
From: jovnc <95868357+jovnc@users.noreply.github.com>
Date: Sat, 7 Mar 2026 11:36:33 +0800
Subject: [PATCH 1/3] Make E2E tests more comprehensive and less flaky

---
 tests/e2e/commands/test_check.py    |  8 ++---
 tests/e2e/commands/test_download.py | 22 ++++++-------
 tests/e2e/commands/test_progress.py | 36 ++++++++++-----------
 tests/e2e/commands/test_setup.py    | 25 ++++++++-------
 tests/e2e/commands/test_verify.py   | 18 ++++++++---
 tests/e2e/conftest.py               | 49 ++++++++++++++++++-----------
 6 files changed, 90 insertions(+), 68 deletions(-)

diff --git a/tests/e2e/commands/test_check.py b/tests/e2e/commands/test_check.py
index 0a20e4f..51cf918 100644
--- a/tests/e2e/commands/test_check.py
+++ b/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")
diff --git a/tests/e2e/commands/test_download.py b/tests/e2e/commands/test_download.py
index a9bf839..d43f591 100644
--- a/tests/e2e/commands/test_download.py
+++ b/tests/e2e/commands/test_download.py
@@ -1,28 +1,28 @@
+import json
 from pathlib import Path
 
 from ..constants import EXERCISE_NAME, HANDS_ON_NAME
 from ..runner import BinaryRunner
 
 
-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)
+def test_download_exercise(runner: BinaryRunner, gitmastery_root: Path) -> None:
+    """download creates the exercise folder with its config and README."""
+    res = runner.run(["download", EXERCISE_NAME], cwd=gitmastery_root)
     res.assert_success()
 
-    exercise_folder = exercises_dir / EXERCISE_NAME
+    exercise_folder = gitmastery_root / EXERCISE_NAME
     assert exercise_folder.is_dir()
 
     exercise_config = exercise_folder / ".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 (exercise_folder / "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)
+def test_download_hands_on(runner: BinaryRunner, gitmastery_root: Path) -> None:
+    """download creates the hands-on folder."""
+    res = runner.run(["download", HANDS_ON_NAME], cwd=gitmastery_root)
     res.assert_success()
 
-    hands_on_folder = exercises_dir / HANDS_ON_NAME
-    assert hands_on_folder.is_dir()
+    assert (gitmastery_root / HANDS_ON_NAME).is_dir()
diff --git a/tests/e2e/commands/test_progress.py b/tests/e2e/commands/test_progress.py
index c6aeb74..d9bf45c 100644
--- a/tests/e2e/commands/test_progress.py
+++ b/tests/e2e/commands/test_progress.py
@@ -1,36 +1,36 @@
+import json
 from pathlib import Path
 
-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
+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()) == []
diff --git a/tests/e2e/commands/test_setup.py b/tests/e2e/commands/test_setup.py
index ea19f50..c0005aa 100644
--- a/tests/e2e/commands/test_setup.py
+++ b/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(gitmastery_root: Path) -> None:
+    """setup creates the expected directory structure, config, and empty progress file."""
+    progress_dir = 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 = 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 (gitmastery_root / ".gitmastery.log").is_file()
diff --git a/tests/e2e/commands/test_verify.py b/tests/e2e/commands/test_verify.py
index af09a1b..8a08c5c 100644
--- a/tests/e2e/commands/test_verify.py
+++ b/tests/e2e/commands/test_verify.py
@@ -1,14 +1,22 @@
+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)
+def test_verify_exercise(runner: BinaryRunner, downloaded_exercise_dir: Path) -> None:
+    """verify runs successfully and writes a progress entry with the expected fields."""
+    res = runner.run(["verify"], cwd=downloaded_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.")
+
+    progress_json = downloaded_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
diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py
index c73f5c0..16ec3e1 100644
--- a/tests/e2e/conftest.py
+++ b/tests/e2e/conftest.py
@@ -3,6 +3,7 @@
 
 import pytest
 
+from .constants import EXERCISE_NAME
 from .utils import rmtree
 from .runner import BinaryRunner
 
@@ -10,34 +11,46 @@
 @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(
+@pytest.fixture
+def 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.
+    Run `setup` in a pytest base temp dir, yield the exercises path.
     """
-    work_dir = tmp_path_factory.mktemp("e2e-tests-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}"
-    )
+    work_dir = tmp_path_factory.mktemp("gitmastery-e2e-test-tmp")
+    res = runner.run(["setup"], cwd=work_dir, stdin_text="\n") # setup with default options
+    res.assert_success()
 
     exercises_path = work_dir / "gitmastery-exercises"
-    assert exercises_path.is_dir(), (
-        f"Expected directory {exercises_path} to exist after setup"
-    )
-
+    assert exercises_path.is_dir()
+    
     try:
         yield exercises_path
     finally:
-        rmtree(work_dir)  # ensure cleanup even if tests fail
+        rmtree(work_dir)
+
+
+@pytest.fixture
+def downloaded_exercise_dir(runner: BinaryRunner, gitmastery_root: Path) -> Path:
+    """
+    Run `download EXERCISE_NAME`, return the exercise dir.
+    """
+    res = runner.run(["download", EXERCISE_NAME], cwd=gitmastery_root)
+    res.assert_success()
+    return gitmastery_root / EXERCISE_NAME
+
+
+@pytest.fixture
+def verified_exercise_dir(runner: BinaryRunner, downloaded_exercise_dir: Path) -> Path:
+    """
+    Run `verify` on the downloaded exercise, return the exercise dir.
+    """
+    res = runner.run(["verify"], cwd=downloaded_exercise_dir)
+    res.assert_success()
+    return downloaded_exercise_dir

From 4316e203a90c7a13b96c1855496b641a0b000ffb Mon Sep 17 00:00:00 2001
From: jovnc <95868357+jovnc@users.noreply.github.com>
Date: Sat, 7 Mar 2026 14:38:16 +0800
Subject: [PATCH 2/3] Address copilot review comments

---
 tests/e2e/conftest.py | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py
index 16ec3e1..3a5b39a 100644
--- a/tests/e2e/conftest.py
+++ b/tests/e2e/conftest.py
@@ -19,19 +19,21 @@ def runner() -> BinaryRunner:
 @pytest.fixture
 def gitmastery_root(
     runner: BinaryRunner, tmp_path_factory: pytest.TempPathFactory
-) -> Generator[Path, None, None]:
+) -> Generator[Path]:
     """
     Run `setup` in a pytest base temp dir, yield the exercises path.
     """
     work_dir = tmp_path_factory.mktemp("gitmastery-e2e-test-tmp")
-    res = runner.run(["setup"], cwd=work_dir, stdin_text="\n") # setup with default options
+
+    # setup with default options
+    res = runner.run(["setup"], cwd=work_dir, stdin_text="\n")
     res.assert_success()
 
-    exercises_path = work_dir / "gitmastery-exercises"
-    assert exercises_path.is_dir()
+    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)
 
@@ -43,7 +45,9 @@ def downloaded_exercise_dir(runner: BinaryRunner, gitmastery_root: Path) -> Path
     """
     res = runner.run(["download", EXERCISE_NAME], cwd=gitmastery_root)
     res.assert_success()
-    return gitmastery_root / EXERCISE_NAME
+    exercise_dir = gitmastery_root / EXERCISE_NAME
+    assert exercise_dir.is_dir()
+    return exercise_dir
 
 
 @pytest.fixture

From 2f67a711143aa78850368d82c0245d9a9872a02c Mon Sep 17 00:00:00 2001
From: jovnc <95868357+jovnc@users.noreply.github.com>
Date: Sat, 7 Mar 2026 14:42:58 +0800
Subject: [PATCH 3/3] Clean up code

---
 tests/e2e/conftest.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py
index 3a5b39a..98475a6 100644
--- a/tests/e2e/conftest.py
+++ b/tests/e2e/conftest.py
@@ -19,7 +19,7 @@ def runner() -> BinaryRunner:
 @pytest.fixture
 def gitmastery_root(
     runner: BinaryRunner, tmp_path_factory: pytest.TempPathFactory
-) -> Generator[Path]:
+) -> Generator[Path, None, None]:
     """
     Run `setup` in a pytest base temp dir, yield the exercises path.
     """
@@ -31,7 +31,7 @@ def gitmastery_root(
 
     gitmastery_root_folder = work_dir / "gitmastery-exercises"
     assert gitmastery_root_folder.is_dir()
-    
+
     try:
         yield gitmastery_root_folder
     finally:
@@ -46,7 +46,6 @@ def downloaded_exercise_dir(runner: BinaryRunner, gitmastery_root: Path) -> Path
     res = runner.run(["download", EXERCISE_NAME], cwd=gitmastery_root)
     res.assert_success()
     exercise_dir = gitmastery_root / EXERCISE_NAME
-    assert exercise_dir.is_dir()
     return exercise_dir