Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions tags_push/.gitmastery-exercise.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"exercise_name": "tags-push",
"tags": [
"git-tag"
],
"requires_git": true,
"requires_github": true,
"base_files": {},
"exercise_repo": {
"repo_type": "remote",
"repo_name": "duty-roster",
"repo_title": "gm-duty-roster",
"create_fork": true,
"init": null
}
}
8 changes: 8 additions & 0 deletions tags_push/README.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the instructions for students to this README?

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# tags-push

The duty-roster repo contains text files that track which people are assigned for duties on which days of the week. This repo is backed up in a remote named production. Apparently, tags in the local repo are not in sync with the tags in your remote.

## Task

1. Push both tags in the local repo to the remote.
2. If any tags are present in the remote production but not in the local repo (i.e., likely result of you previously deleting them in the local repo but forgetting to delete them in the remote repo), delete them in the remote.
Empty file added tags_push/__init__.py
Empty file.
11 changes: 11 additions & 0 deletions tags_push/download.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from exercise_utils.cli import run_command
from exercise_utils.git import tag

def setup(verbose: bool = False):
run_command(["git", "remote", "rename", "origin", "production"], verbose)
tag("beta", verbose)
run_command(["git", "push", "production", "--tags"], verbose)
run_command(["git", "tag", "-d", "beta"], verbose)

run_command(["git", "tag", "v1.0", "HEAD~4"], verbose)
run_command(["git", "tag", "-a", "v2.0", "HEAD~1", "-m", "First stable roster"], verbose)
Empty file added tags_push/tests/__init__.py
Empty file.
6 changes: 6 additions & 0 deletions tags_push/tests/specs/base.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
initialization:
steps:
- type: commit
empty: true
message: Empty commit
id: start
121 changes: 121 additions & 0 deletions tags_push/tests/test_verify.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to include the case where all checks fail so we're verifying against multiple comments.

Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import json
from pathlib import Path
from unittest.mock import patch

import pytest
from git.repo import Repo

from git_autograder import (
GitAutograderExercise,
GitAutograderStatus,
GitAutograderTestLoader,
GitAutograderWrongAnswerException,
assert_output)

from ..verify import (
IMPROPER_GH_CLI_SETUP,
TAG_1_NAME,
TAG_2_NAME,
TAG_DELETE_NAME,
TAG_1_MISSING,
TAG_2_MISSING,
TAG_DELETE_NOT_REMOVED,
verify)

REPOSITORY_NAME = "tags-push"

loader = GitAutograderTestLoader(__file__, REPOSITORY_NAME, verify)


# NOTE: This exercise is a special case where we do not require repo-smith. Instead,
# we directly mock function calls to verify that all branches are covered for us.


# TODO: The current tooling isn't mature enough to handle mock GitAutograderExercise in
# cases like these. We would ideally need some abstraction rather than creating our own.


@pytest.fixture
def exercise(tmp_path: Path) -> GitAutograderExercise:
repo_dir = tmp_path / "ignore-me"
repo_dir.mkdir()

Repo.init(repo_dir)
with open(tmp_path / ".gitmastery-exercise.json", "a") as config_file:
config_file.write(
json.dumps(
{
"exercise_name": "tags-push",
"tags": [],
"requires_git": True,
"requires_github": True,
"base_files": {},
"exercise_repo": {
"repo_type": "local",
"repo_name": "ignore-me",
"init": True,
"create_fork": None,
"repo_title": None,
},
"downloaded_at": None,
}
)
)

exercise = GitAutograderExercise(exercise_path=tmp_path)
return exercise


def test_pass(exercise: GitAutograderExercise):
with (
patch("tags_push.verify.get_username", return_value="dummy"),
patch("tags_push.verify.get_remote_tags", return_value=[TAG_1_NAME, TAG_2_NAME]),
):
output = verify(exercise)
assert_output(output, GitAutograderStatus.SUCCESSFUL)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not wrong, it seems like coding standard in this repository is 2 new lines between each method. This is based on my observation of other files within this codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, let me fix that.


def test_improper_gh_setup(exercise: GitAutograderExercise):
with (
patch("tags_push.verify.get_username", return_value=None),
patch("tags_push.verify.get_remote_tags", return_value=[TAG_1_NAME, TAG_2_NAME]),
pytest.raises(GitAutograderWrongAnswerException, match=IMPROPER_GH_CLI_SETUP),
):
verify(exercise)


def test_beta_present(exercise: GitAutograderExercise):
with (
patch("tags_push.verify.get_username", return_value="dummy"),
patch("tags_push.verify.get_remote_tags", return_value=[TAG_1_NAME, TAG_2_NAME, TAG_DELETE_NAME]),
pytest.raises(GitAutograderWrongAnswerException, match=TAG_DELETE_NOT_REMOVED),
):
verify(exercise)

def test_tag_1_absent(exercise: GitAutograderExercise):
with (
patch("tags_push.verify.get_username", return_value="dummy"),
patch("tags_push.verify.get_remote_tags", return_value=[TAG_2_NAME]),
pytest.raises(GitAutograderWrongAnswerException, match=TAG_1_MISSING),
):
verify(exercise)


def test_tag_2_absent(exercise: GitAutograderExercise):
with (
patch("tags_push.verify.get_username", return_value="dummy"),
patch("tags_push.verify.get_remote_tags", return_value=[TAG_1_NAME]),
pytest.raises(GitAutograderWrongAnswerException, match=TAG_2_MISSING),
):
verify(exercise)


def test_all_wrong(exercise: GitAutograderExercise):
with (
patch("tags_push.verify.get_username", return_value="dummy"),
patch("tags_push.verify.get_remote_tags", return_value=[TAG_DELETE_NAME]),
pytest.raises(GitAutograderWrongAnswerException) as exception,
):
verify(exercise)
Comment on lines +117 to +119
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment


assert exception.value.message == [TAG_1_MISSING, TAG_2_MISSING, TAG_DELETE_NOT_REMOVED]
68 changes: 68 additions & 0 deletions tags_push/verify.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import os
import subprocess
from typing import List, Optional

from git_autograder import (
GitAutograderOutput,
GitAutograderExercise,
GitAutograderStatus,
)

IMPROPER_GH_CLI_SETUP = "Your Github CLI is not setup correctly"

TAG_1_NAME = "v1.0"
TAG_2_NAME = "v2.0"
TAG_DELETE_NAME = "beta"

TAG_1_MISSING = f"Tag {TAG_1_NAME} is missing, did you push it to the remote?"
TAG_2_MISSING = f"Tag {TAG_2_NAME} is missing, did you push it to the remote?"
TAG_DELETE_NOT_REMOVED = f"Tag {TAG_DELETE_NAME} is still on the remote!"


def run_command(command: List[str]) -> Optional[str]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a run_command and run_command_with_no_exit util under exercise_util, do you think we can reuse those instead? Or is there something specific in this function that makes this function necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a convention to not import anything from exercise_utils for the autograding (See fork_repo and clone_repo) and to define the functions in the verify namespace. Thus, I follow a similar convention.

try:
result = subprocess.run(
command,
capture_output=True,
text=True,
check=True,
env=dict(os.environ, **{"GH_PAGER": "cat"}),
)
return result.stdout.strip()
except subprocess.CalledProcessError:
return None


def get_username() -> Optional[str]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_username and get_remote_tags seems to be reusable components that can be placed under exercise_utils, do you think we should move it there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See reply to run_command

return run_command(["gh", "api", "user", "-q", ".login"])


def get_remote_tags(username: str) -> List[str]:
raw_tags = run_command(["git", "ls-remote", "--tags"])
return [line.split("/")[2] for line in raw_tags.strip().splitlines()]


def verify(exercise: GitAutograderExercise) -> GitAutograderOutput:
username = get_username()
if username is None:
raise exercise.wrong_answer([IMPROPER_GH_CLI_SETUP])
Comment on lines +46 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all fairness, this should not be necessary because the requires_github check already covers it, but I get the current constraint. Happy to keep this for now.


tag_names = get_remote_tags(username)

comments = []

if TAG_1_NAME not in tag_names:
comments.append(TAG_1_MISSING)

if TAG_2_NAME not in tag_names:
comments.append(TAG_2_MISSING)

if TAG_DELETE_NAME in tag_names:
comments.append(TAG_DELETE_NOT_REMOVED)

if comments:
raise exercise.wrong_answer(comments)

return exercise.to_output(
["Wonderful! You have successfully synced the local tags with the remote tags!"],
GitAutograderStatus.SUCCESSFUL)