Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests for git / github module - adventures in patching and mocking #445

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions .github/workflows/code-cov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jobs:
run: |
pip install pytest
pip install pytest-cov
pip install pytest-subprocess
lwasser marked this conversation as resolved.
Show resolved Hide resolved
pip install -e .
pytest --cov=./ --cov-report=xml
- name: Upload coverage to Codecov
Expand Down
4 changes: 4 additions & 0 deletions abcclassroom/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ def remote_repo_exists(org, repository, token=None):
return True


# TODO: do neither github python package wrap clone for us?
def clone_repo(organization, repo, dest_dir):
"""Clone `repository` from `org` into a sub-directory in `directory`.

Expand Down Expand Up @@ -435,6 +436,9 @@ def check_student_repo_exists(org, course, student, token=None):
repository = "{}-{}".format(course, student)
g.repository(org, repository)

# TODO: this raises github3.exceptions.NotFoundError: 404 Not Found
# It might be better to capture the specific exception and raise a more
# helpful error?
except Exception as e:
raise e

Expand Down
363 changes: 307 additions & 56 deletions abcclassroom/tests/test_github.py
Original file line number Diff line number Diff line change
@@ -1,60 +1,311 @@
# Tests for git and github methods
from pathlib import Path

import abcclassroom.github as abcgit


def test_init_and_commit(default_config, tmp_path):
"""
Tests that we can create a directory, initialize it as a git repo,
and commit some changes.
"""
repo_dir = Path(tmp_path, "init-and-commit")
repo_dir.mkdir()
a_file = Path(repo_dir, "testfile.txt")
a_file.write_text("Some text")
abcgit.init_and_commit(repo_dir)
assert Path(repo_dir, ".git").exists()
git_return = abcgit._call_git("log", directory=repo_dir)
assert git_return.stdout.startswith("commit")


def test_master_branch_to_main_repeatedly(tmp_path):
"""
Tests that we can sucessfully change the default master branch to
main, and nothing bad happends if we try and do it again
"""
repo_dir = Path(tmp_path, "change-master")
repo_dir.mkdir()
abcgit.git_init(repo_dir)

# change the default branch name
abcgit._master_branch_to_main(repo_dir)
# in order to test that main exists, we need to add some commits
a_file = Path(repo_dir, "testfile.txt")
a_file.write_text("Some text")
commit_msg = "commit some things"
abcgit.commit_all_changes(repo_dir, msg=commit_msg)

# now test the existance of main
abcgit._call_git(
"show-ref",
"--quiet",
"--verify",
"refs/heads/main",
directory=repo_dir,
# Tests for github script

import pytest
import github3 as gh3
import abcclassroom.github as github
import os
from unittest import mock


# Creating an OrganizationObject that will run instead of
# github3.create_repository
class OrganizationObject(object):
# TODO: Should this have a static decorator on it?
# @staticmethod
def create_repository(self, repository):
if repository == "test_repo":
return True
Copy link
Author

Choose a reason for hiding this comment

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

Why is code cov running test coverage on mock objects? do we have to decorate these in some way?



# Creating a MockResponse object to be returned when trying to log into github
class MockResponse(object):
def repository(self, org, repository, token=None):
if org == "earthlab" and repository == "abc-classroom":
pass
elif org == "earthlab" and repository == "test-student":
pass
else:
raise Exception
Copy link
Author

Choose a reason for hiding this comment

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

So there is a specific exception raised. we prbably want to capture that but need to decide on what package first.


def organization(self, org):
return OrganizationObject()


# Function to return Mock Response for login.
def mock_login(token=None):
return MockResponse()


# Function to replace check_ssh
def mock_check_ssh():
"""stuff here"""
# I'm not sure if this is really what we want?
# We could also mock up an ssh key in a temp directory?
return print("Pretending ssh is all setup nicely?")


# so if you run the check repo g.repository(org, repository) it returns the
# text below if the repo exists. but i don't know if that is the return from
# the package OR the api
# g.repository(org, repository)
# Out[57]: <Repository [earthlab/earthpy]>


def test_remote_repo_exists_pass(monkeypatch):
"""If the remote repo exists, return True"""
# Creating the fake function that will replace gh3.login with mock_login.
monkeypatch.setattr(gh3, "login", mock_login)
# Running this function with the newly faked function inside of it.
assert github.remote_repo_exists("earthlab", "abc-classroom")


def test_remote_repo_exists_fail(monkeypatch):
"""Test that a remote repository that doesn't exist fails."""
monkeypatch.setattr(gh3, "login", mock_login)
assert not github.remote_repo_exists("bad_org", "bad_repo")


# NOTE: Updated to run git status so it works universally
def test_call_git_status(fake_process):
"""Testing that _call_git helper function works"""
# When a function uses
# subprocess.run instead of called gh3 directly, it's difficult to
# fake with monkeypatch. There's a package developed specifically for this
# called pytest_process and has a fake_process object. This objects that
# allows subprocesses to be mocked.

# TODO - is this really mocking the subprocess call?
fake_process.register_subprocess(["git", "status"], stdout=["On branch"])
lwasser marked this conversation as resolved.
Show resolved Hide resolved
ret = github._call_git("status")
assert "On branch" in ret.stdout


# TODO: This test assumes a dir called "test_dir" exists and it doesnt exist.
# Is this a space for a fixture / temp dir setup?


@pytest.fixture()
def example_student_repo():
"""A fixture with an example student repo."""
Copy link
Author

@lwasser lwasser Oct 26, 2021

Choose a reason for hiding this comment

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

this is the start of a fake repo fixture that i can make - here i would init the repo and drop it wherever the test needed it and then we could run checks on it.

# TODO can a fixture take a variable like a function can?
# or should this only create the git dir and let the calling function worry
# about changing the directory?
sample_dir = os.path.join("assignment-1", "course-test-student")
if not os.path.isdir(sample_dir):
os.makedirs(sample_dir)
# do we need this to be a git repo? probably would be useful for other
# tests


# def mock_clone(example_student_repo, a_directory, tmp_path, test):
lwasser marked this conversation as resolved.
Show resolved Hide resolved
# """Takes a repo that has been mock cloned and places it in the intended
# directory. """
# # When it's mocking it seems to replace each variable in the function in
# # a weird unexpected way. - example_student_repo = '-C', a_directory =
# # 'assignment-1', tmp_path = 'clone', test =
# # '[email protected]:earthlab/course-test-student.git'
#
# # TODO: make this so it's drops the example dir in the dir_path location
# #tmp_path = "."
# # Create the rep
# #a_directory = "test_dir"
# clone_path = os.path.join(tmp_path, a_directory)
# print("clone_path is", clone_path)
# os.chdir(clone_path)
# # Move repo to location
# return example_student_repo()

# stackoverflow.com/questions/25692440/mocking-a-subprocess-call-in-python


@mock.patch("subprocess.run")
def test_clone_repo_pass2(
mock_subproc_run, monkeypatch, example_student_repo, capsys
Copy link
Author

Choose a reason for hiding this comment

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

here i'm using pytest monkeypatch AND mock_subproc_Run from unittest
is that reasonable? i like mock but monkeypatch is easier for me. but mock seems to better handle subprocess stuff

):

# This fixture will drop an example git
# repo for us to check that it exists assignment-1/course-test-student"
example_student_repo
# Replace check_github_ssh with a pass (Assume that works - is that ok??)
monkeypatch.setattr(github, "check_git_ssh", mock_check_ssh)

# Mock the subprocess call
process_mock = mock.Mock()
lwasser marked this conversation as resolved.
Show resolved Hide resolved
attrs = {"communicate.return_value": ("output", "error")}
process_mock.configure_mock(**attrs)
mock_subproc_run.return_value = process_mock
github.clone_repo(
organization="earthlab", repo="earthpy", dest_dir="assignment-1"
)
captured = capsys.readouterr()
lines = captured.out.splitlines()
# assert captured.out starts with "cloning: git@github"
# print("HI:", lines[1], captured.out)
# TODO do we also want to create a mock repo for it to check?
assert lines[1].startswith("cloning: git@github")


# TODO: ok i just tested this. it's definitely not skipping the clone.
# def test_clone_repo_pass(fake_process, monkeypatch, example_student_repo):
# """Test that a repo is correctly cloned to the right location."""
#
# # Replace check_github_ssh with a pass (Assume that works - is that ok??)
# # Here i'm confused because i would think we're replacing _call_git with
# # a mock function so it doesn't try to clone
# monkeypatch.setattr(github, "check_git_ssh", mock_check_ssh)
#
# # it is actually cloning hte data. we want to monkey patch around the
# # entire function i think or somehow use the faek subprocess to fake the
# # call
# git_commands = [
# "git",
# "-C",
# "assignment-1",
# "clone",
# "[email protected]:earthlab/abc-classroom.git",
# ]
# fake_process.register_subprocess(git_commands,
# stderr=b"Cloning into 'abc-classroom'...\n" )
# # Not sure why it keeps telling me it's not registered so adding this
# fake_process.allow_unregistered(True)
# # Create the dir needed for the assignment (clone doesnt do this)
# # once this works we can use a tmp path fixture for it
# example_student_repo
# # Run the fake process - i thought we're testing clone_repo
# process = github.clone_repo(organization="earthlab",
# repo="test",
# dest_dir="assignment-1")
# # Create the new directory
# out, _ = process.communicate()
# print(out)
# # # Replace _call_git with the returned repo
# # monkeypatch.setattr(github, "_call_git", mock_clone)
# # organization, repo, dest_dir
#
# assert os.path.isdir('assignment-1')


# i suspect that this needs a fixture or object that returns a repository
# in a new directory.
# then i guess we can test that the repo contains a .git directory?
# and maybe some files?
# def test_clone_repo_pass1(fake_process, tmp_path):
# """Test that a cloned repo is saved in the correct location."""
# os.chdir(tmp_path)
# # TODO: This forces subprocess to work BUT i'm guessing it won't work on
# # CI because SSH is not setup. We may want a fixture that creates an envt
# # with a ssh key available?
# fake_process.allow_unregistered(True)
# fake_process.register_subprocess(
# [
# "git",
# "clone" "[email protected]:earthlab/abc-classroom.git",
# ],
# stderr=b"Cloning into 'abc-classroom'...\n",
# )
# try:
# github.clone_repo("earthlab", "abc-classroom", ".")
# # TODO: should we do a dir check here to ensure that correct dir is
# # actually created and contains expected files??
# assert True
# except RuntimeError:
# assert False


#
#
# def test_create_repo_pass(monkeypatch):
# """Test that creating a new repository works."""
# monkeypatch.setattr(gh3, "login", mock_login)
# try:
# github.create_repo(org="earthlab",repository="test_repo", token=None)
# assert True
# except RuntimeError:
# assert False


# This is the old tests
# from pathlib import Path
#
# import abcclassroom.github as abcgit
#
#
# def test_init_and_commit(default_config, tmp_path):
# """
# Tests that we can create a directory, initialize it as a git repo,
# and commit some changes.
# """
# repo_dir = Path(tmp_path, "init-and-commit")
# repo_dir.mkdir()
# a_file = Path(repo_dir, "testfile.txt")
# a_file.write_text("Some text")
# abcgit.init_and_commit(repo_dir)
# assert Path(repo_dir, ".git").exists()
# git_return = abcgit._call_git("log", directory=repo_dir)
# assert git_return.stdout.startswith("commit")
#
#
# def test_master_branch_to_main_repeatedly(tmp_path):
# """
# Tests that we can sucessfully change the default master branch to
# main, and nothing bad happends if we try and do it again
# """
# repo_dir = Path(tmp_path, "change-master")
# repo_dir.mkdir()
# abcgit.git_init(repo_dir)
#
# # change the default branch name
# abcgit._master_branch_to_main(repo_dir)
# # in order to test that main exists, we need to add some commits
# a_file = Path(repo_dir, "testfile.txt")
# a_file.write_text("Some text")
# commit_msg = "commit some things"
# abcgit.commit_all_changes(repo_dir, msg=commit_msg)
#
# # now test the existance of main
# abcgit._call_git(
# "show-ref",
# "--quiet",
# "--verify",
# "refs/heads/main",
# directory=repo_dir,
# )
#
# # trying again should also work without error
# abcgit._master_branch_to_main(repo_dir)
#
#
# def test_master_branch_to_main_no_commits(tmp_path):
# """
# Tests that changing the name of the master branch in a initialized
# repo without commits works.
# """
# repo_dir = Path(tmp_path, "change-master-no-commits")
# repo_dir.mkdir()
# abcgit.git_init(repo_dir)
# abcgit._master_branch_to_main(repo_dir)


# trying again should also work without error
abcgit._master_branch_to_main(repo_dir)
# TODO: https://github.com/earthlab/abc-classroom/issues/361 this function
# isnt' in karen's list of functions to test
# def test_check_student_repo_exists_pass(monkeypatch):
# """Testing that checking a student repo works"""
# monkeypatch.setattr(gh3, "login", mock_login)
# try:
# github.check_student_repo_exists("earthlab", "test", "student")
# assert True
# except RuntimeError:
# assert False


def test_master_branch_to_main_no_commits(tmp_path):
"""
Tests that changing the name of the master branch in a initialized
repo without commits works.
"""
repo_dir = Path(tmp_path, "change-master-no-commits")
repo_dir.mkdir()
abcgit.git_init(repo_dir)
abcgit._master_branch_to_main(repo_dir)
# TODO: similar to above - this isn't in the list of functions to test.
# This function used to be called in MAIN but is now commented out
# SO i think we should drop these tests
#
# def test_check_student_repo_fail(monkeypatch):
# """A student repo that doesn't exist should fail"""
# monkeypatch.setattr(gh3, "login", mock_login)
# # TODO if we use github3 there is a speciicaly Not FoundError exception
# # that should be captures here
# with pytest.raises(Exception):
# github.check_student_repo_exists("earthlab", "not", "student")
Loading