[grocery-shopping] Improve verify logic (#232)

# Exercise Review

## Exercise Discussion

Fixes #225

## Checklist

- [ ] If you require a new remote repository on the `Git-Mastery`
organization, have you [created a
request](https://github.com/git-mastery/exercises/issues/new?template=request_exercise_repository.yaml)
for it?
- [x] Have you written unit tests using
[`repo-smith`](https://github.com/git-mastery/repo-smith) to validate
the exercise grading scheme?
- [x] Have you tested your changes using the instructions posted?
- [x] Have you verified that this exercise does not already exist or is
not currently in review?
- [ ] Did you introduce a new grading mechanism that should belong to
[`git-autograder`](https://github.com/git-mastery/git-autograder)?
- [ ] Did you introduce a new dependency that should belong to
[`app`](https://github.com/git-mastery/app)?

Author: desmondwong1215

grocery_shopping/test_verify.py

@@ -10,7 +10,14 @@
 from git_autograder import GitAutograderStatus
 from repo_smith.repo_smith import RepoSmith
 
-from .verify import EMPTY_COMMITS, NO_ADD, NO_REMOVE, WRONG_FILE, verify
+from .verify import (
+    ADD_NOT_COMMITTED, 
+    NO_ADD, 
+    NO_REMOVE, 
+    REMOVE_NOT_COMMITTED, 
+    SHOPPING_LIST_FILE_MISSING, 
+    verify
+)
 
 REPOSITORY_NAME = "grocery-shopping"
 
@@ -20,7 +27,6 @@
 @contextmanager
 def base_setup() -> Iterator[Tuple[GitAutograderTest, RepoSmith]]:
     with loader.start() as (test, rs):
-        rs.files.create_or_update("README.md", "Hello world")
         rs.files.create_or_update(
             "shopping-list.txt",
             """
@@ -31,7 +37,7 @@ def base_setup() -> Iterator[Tuple[GitAutograderTest, RepoSmith]]:
             - Ham
             """,
         )
-        rs.git.add(["README.md", "shopping-list.txt"])
+        rs.git.add(["shopping-list.txt"])
         rs.git.commit(message="Initial commit")
         rs.helper(GitMasteryHelper).create_start_tag()
 
@@ -40,23 +46,35 @@ def base_setup() -> Iterator[Tuple[GitAutograderTest, RepoSmith]]:
 
 def test_no_changes():
     with base_setup() as (test, rs):
-        rs.git.commit(message="Commit", allow_empty=True)
+        output = test.run()
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [NO_ADD, NO_REMOVE])
+
+
+def test_add_new_file():
+    with base_setup() as (test, rs):
+        rs.files.create_or_update("new-file.txt", "New file content")
 
         output = test.run()
-        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [EMPTY_COMMITS])
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [NO_ADD, NO_REMOVE])
 
 
-def test_wrong_file():
+def test_delete_file():
     with base_setup() as (test, rs):
-        rs.files.create_or_update("README.md", "Goodbye")
-        rs.git.add(all=True)
-        rs.git.commit(message="Update README.md")
+        rs.files.delete("shopping-list.txt")
 
         output = test.run()
-        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [WRONG_FILE])
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [SHOPPING_LIST_FILE_MISSING])
 
 
-def test_only_edit():
+def test_add_only():
+    with base_setup() as (test, rs):
+        rs.files.append("shopping-list.txt", "- Chicken\n",)
+
+        output = test.run()
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [NO_REMOVE])
+
+
+def test_remove_only():
     with base_setup() as (test, rs):
         rs.files.create_or_update(
             "shopping-list.txt",
@@ -65,27 +83,14 @@ def test_only_edit():
             - Eggs
             - Bread
             - Apples
-            - Ham1
             """,
         )
-        rs.git.add(all=True)
-        rs.git.commit(message="Update shopping list")
-
-        output = test.run()
-        assert_output(output, GitAutograderStatus.SUCCESSFUL)
-
-
-def test_no_add():
-    with base_setup() as (test, rs):
-        rs.files.create_or_update("shopping-list.txt", "- Milk")
-        rs.git.add(all=True)
-        rs.git.commit(message="Update shopping list")
 
         output = test.run()
         assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [NO_ADD])
 
 
-def test_no_remove():
+def test_changes_not_committed():
     with base_setup() as (test, rs):
         rs.files.create_or_update(
             "shopping-list.txt",
@@ -94,19 +99,20 @@ def test_no_remove():
             - Eggs
             - Bread
             - Apples
-            - Ham
             - Chicken
             """,
         )
-        rs.git.add(all=True)
-        rs.git.commit(message="Update shopping list")
 
         output = test.run()
-        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [NO_REMOVE])
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [ADD_NOT_COMMITTED, REMOVE_NOT_COMMITTED])
 
 
-def test_one_shot():
+def test_add_committed_only():
     with base_setup() as (test, rs):
+        rs.files.append("shopping-list.txt", "- Chicken\n",)
+        rs.git.add(all=True)
+        rs.git.commit(message="Add Chicken to shopping list")
+
         rs.files.create_or_update(
             "shopping-list.txt",
             """
@@ -115,16 +121,14 @@ def test_one_shot():
             - Bread
             - Apples
             - Chicken
-            """,
+            """
         )
-        rs.git.add(all=True)
-        rs.git.commit(message="Update shopping list")
 
         output = test.run()
-        assert_output(output, GitAutograderStatus.SUCCESSFUL)
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [REMOVE_NOT_COMMITTED])
 
 
-def test_complex():
+def test_remove_committed_only():
     with base_setup() as (test, rs):
         rs.files.create_or_update(
             "shopping-list.txt",
@@ -133,11 +137,19 @@ def test_complex():
             - Eggs
             - Bread
             - Apples
-            """,
+            """
         )
         rs.git.add(all=True)
-        rs.git.commit(message="Delete item")
+        rs.git.commit(message="Add Chicken to shopping list")
+
+        rs.files.append("shopping-list.txt", "- Chicken\n",)
 
+        output = test.run()
+        assert_output(output, GitAutograderStatus.UNSUCCESSFUL, [ADD_NOT_COMMITTED])
+
+
+def test_successful_change():
+    with base_setup() as (test, rs):
         rs.files.create_or_update(
             "shopping-list.txt",
             """
@@ -149,7 +161,7 @@ def test_complex():
             """,
         )
         rs.git.add(all=True)
-        rs.git.commit(message="Add item")
+        rs.git.commit(message="Update shopping list")
 
         output = test.run()
         assert_output(output, GitAutograderStatus.SUCCESSFUL)

grocery_shopping/verify.py

@@ -1,3 +1,4 @@
+import os
 from typing import List
 
 from git_autograder import (
@@ -6,33 +7,56 @@
     GitAutograderStatus,
 )
 
-EMPTY_COMMITS = "All commits are empty."
-NO_DIFF = "There are no changes made to shopping-list.txt."
 NO_ADD = "There are no new grocery list items added to the shopping list."
 NO_REMOVE = "There are no grocery list items removed from the shopping list."
-WRONG_FILE = "You haven't edited shopping-list.txt."
+SHOPPING_LIST_FILE_MISSING = "The shopping-list.txt file should not be deleted."
+ADD_NOT_COMMITTED = "New grocery list items added to shopping-list.txt are not committed."
+REMOVE_NOT_COMMITTED = "Grocery list items removed from shopping-list.txt are not committed."
 
 ORIGINAL_SHOPPING_LIST = {"Milk", "Eggs", "Bread", "Apples", "Ham"}
 
 
 def verify(exercise: GitAutograderExercise) -> GitAutograderOutput:
     comments: List[str] = []
+    repo_root = exercise.exercise_path
+    repo_folder = exercise.config.exercise_repo.repo_name
+    work_dir = os.path.join(repo_root, repo_folder)
 
-    main_branch = exercise.repo.branches.branch("main")
+    shopping_list_file_path = os.path.join(work_dir, "shopping-list.txt")
+    if not os.path.exists(shopping_list_file_path):
+        raise exercise.wrong_answer([SHOPPING_LIST_FILE_MISSING])
+    
+    with open(shopping_list_file_path, "r", encoding="utf-8") as f:
+        content = f.read()
+    
+    current_shopping_list = {
+        line[2:].strip() 
+        for line in content.splitlines() 
+        if line.startswith("- ")
+    }
+
+    added_items = current_shopping_list.difference(ORIGINAL_SHOPPING_LIST)
+    deleted_items = ORIGINAL_SHOPPING_LIST.difference(current_shopping_list)
+
+    if not added_items:
+        comments.append(NO_ADD)
 
-    # Verify that not all commits are empty
-    if not main_branch.has_non_empty_commits():
-        raise exercise.wrong_answer([EMPTY_COMMITS])
+    if not deleted_items:
+        comments.append(NO_REMOVE)
 
-    # Check if they edited the shopping-list.md at least once
-    if not main_branch.has_edited_file("shopping-list.txt"):
-        raise exercise.wrong_answer([WRONG_FILE])
+    if comments:
+        raise exercise.wrong_answer(comments)
+    
+    main_branch = exercise.repo.branches.branch("main")
+
+    if not main_branch.user_commits:
+        raise exercise.wrong_answer([ADD_NOT_COMMITTED, REMOVE_NOT_COMMITTED])
 
     shopping_list_blob = (
         main_branch.latest_user_commit.commit.tree / "shopping-list.txt"
     )
     current_shopping_list = {
-        line[2:]
+        line[2:].strip()
         for line in shopping_list_blob.data_stream.read().decode().split("\n")
         if line.startswith("- ")
     }
@@ -41,10 +65,10 @@ def verify(exercise: GitAutograderExercise) -> GitAutograderOutput:
     deleted_items = ORIGINAL_SHOPPING_LIST.difference(current_shopping_list)
 
     if not added_items:
-        comments.append(NO_ADD)
+        comments.append(ADD_NOT_COMMITTED)
 
     if not deleted_items:
-        comments.append(NO_REMOVE)
+        comments.append(REMOVE_NOT_COMMITTED)
 
     if comments:
         raise exercise.wrong_answer(comments)