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 in Github and Gitlab integrations should actually clone a repo #15241

Open
WWM-jschuba opened this issue Sep 5, 2024 · 0 comments
Open
Labels
development Tech debt, refactors, CI, tests, and other related work. enhancement An improvement of an existing feature

Comments

@WWM-jschuba
Copy link
Contributor

Describe the current behavior

Currently, the tests for prefect-github and prefect-github get_directory() function only test part of the functionality. They do not actually clone a repo, which is the major purpose of that function.

The tests mock up a temp directory, with files inside. Because that directory is not empty, the git clone command silently fails. The remainder of the function successfully copies the pre-made files, and returns a full destination directory.

This appears to be the intention of the test writer. The test might be slow if it actually cloned a git repo, however, that critical component remains untested.

As a result, several bugs made it into production. Some examples:
#11279
#13180
#15206
#15236

Describe the proposed behavior

Create new tests in prefect-github and prefect-gitlab that clone a repo, and verify that it was cloned successfully. Example is below.

We need to decide what repo to clone. The prefect repo is quite large. We might also want to test the cloning of a private repo with secret keys.

async def test_get_directory_clones_a_repo(self, monkeypatch):  # noqa
        """Check that `get_directory` is able to clone a repo"""  # noqa

        with TemporaryDirectory() as tmp_dst:

            g = GitHubRepository(
                repository_url="https://github.com/PrefectHQ/prefect.git",
            )
            await g.get_directory(local_path=tmp_dst)

            # Verify that we cloned the repo
            dir_list = set(os.listdir(tmp_dst))
            assert 'README.md' in dir_list  # The repo should have a README at the root
            assert '.git' in dir_list  # The repo should have a .git directory

Example Use

N/A. This is a proposal for new tests.

Additional context

No response

@WWM-jschuba WWM-jschuba added the enhancement An improvement of an existing feature label Sep 5, 2024
@cicdw cicdw added the development Tech debt, refactors, CI, tests, and other related work. label Sep 5, 2024
@cicdw cicdw added this to OSS Backlog Sep 5, 2024
@cicdw cicdw moved this to Backlog in OSS Backlog Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Tech debt, refactors, CI, tests, and other related work. enhancement An improvement of an existing feature
Projects
Status: Backlog
Development

No branches or pull requests

2 participants