From b74477834f713ae942b9b15b12523939dba1a563 Mon Sep 17 00:00:00 2001 From: Dom Sekotill Date: Sat, 9 Apr 2022 01:43:21 +0100 Subject: [PATCH 1/5] Remove unnecessary Ref constants from test fixtures --- .../unit/gitlab_ci/pre_commit_run/fixtures.py | 15 +++----- tests/unit/gitlab_ci/pre_commit_run/tests.py | 36 +++++++++---------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/tests/unit/gitlab_ci/pre_commit_run/fixtures.py b/tests/unit/gitlab_ci/pre_commit_run/fixtures.py index aef2a30..933ec84 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/fixtures.py +++ b/tests/unit/gitlab_ci/pre_commit_run/fixtures.py @@ -28,11 +28,6 @@ from project_templates.resources import TempDir Ref = NewType("Ref", str) Sha = NewType("Sha", str) -MAIN = Ref("main") -FORK = Ref("fork") -LCA = Ref("lca") -ORPHAN = Ref("orphan") - class Branch: """ @@ -80,7 +75,7 @@ class Repo: def __str__(self) -> str: return self.url - def init(self, branch: str = MAIN) -> None: + def init(self, branch: str = "main") -> None: """ Initialise (git-init) a bare repository, with "branch" checked out """ @@ -151,15 +146,15 @@ def setup_fixture_repo(repo: Repo) -> dict[Ref, Branch]: random.seed(1) root = create_commit(repo) - main = Branch(repo, MAIN, root) - fork = Branch(repo, FORK, root) - lca = Branch(repo, LCA, root) + main = Branch(repo, "main", root) + fork = Branch(repo, "fork", root) + lca = Branch(repo, "lca", root) lca.head = create_commit_path(repo, root, 5) main.head = create_commit_path(repo, lca.head, 5) fork.head = create_commit_path(repo, lca.head, 5) - orphan = Branch(repo, ORPHAN, create_commit(repo)) + orphan = Branch(repo, "orphan", create_commit(repo)) orphan.head = create_commit_path(repo, orphan.root, 5) return {b.name: b for b in (main, fork, lca, orphan)} diff --git a/tests/unit/gitlab_ci/pre_commit_run/tests.py b/tests/unit/gitlab_ci/pre_commit_run/tests.py index dc087bd..0adc4e5 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/tests.py +++ b/tests/unit/gitlab_ci/pre_commit_run/tests.py @@ -24,10 +24,6 @@ from pathlib import Path from typing import Iterable from typing import Mapping -from .fixtures import FORK -from .fixtures import LCA -from .fixtures import MAIN -from .fixtures import ORPHAN from .fixtures import Branch from .fixtures import Ref from .fixtures import Repo @@ -102,11 +98,11 @@ class PreCommitRunTests(unittest.TestCase): """ Check that calling with CI_COMMIT_BEFORE_SHA set ensures the commit is available """ - from_sha = self.repo.get_sha(f"{MAIN}~4") - to_sha = self.repo.get_sha(MAIN) + from_sha = self.repo.get_sha("main~4") + to_sha = self.repo.get_sha("main") args = self.run_script( - MAIN, + "main", CI_COMMIT_BEFORE_SHA=from_sha, ) @@ -122,14 +118,14 @@ class PreCommitRunTests(unittest.TestCase): Check that calling in merge request pipelines finds the LCA with the target branch """ args = self.run_script( - FORK, + "fork", CI_PIPELINE_SOURCE="merge_request_event", ) expect = [ "run", - f"--from-ref={self.branches[LCA].head}", - f"--to-ref={self.repo.get_sha(FORK)}", + f"--from-ref={self.repo.get_sha('lca')}", + f"--to-ref={self.repo.get_sha('fork')}", ] self.assertListEqual(expect, args) @@ -139,12 +135,12 @@ class PreCommitRunTests(unittest.TestCase): CI_COMMIT_BRANCH != CI_DEFAULT_BRANCH """ - args = self.run_script(FORK) + args = self.run_script("fork") expect = [ "run", - f"--from-ref={self.branches[LCA].head}", - f"--to-ref={self.repo.get_sha(FORK)}", + f"--from-ref={self.repo.get_sha('lca')}", + f"--to-ref={self.repo.get_sha('fork')}", ] self.assertListEqual(expect, args) @@ -154,7 +150,7 @@ class PreCommitRunTests(unittest.TestCase): CI_COMMIT_BRANCH == CI_DEFAULT_BRANCH """ - args = self.run_script(MAIN) + args = self.run_script("main") expect = ["run", "--all-files"] self.assertListEqual(expect, args) @@ -163,7 +159,7 @@ class PreCommitRunTests(unittest.TestCase): """ Check that pushing to a new orphan branch check all files """ - args = self.run_script(ORPHAN) + args = self.run_script("orphan") expect = ["run", "--all-files"] self.assertListEqual(expect, args) @@ -172,7 +168,7 @@ class PreCommitRunTests(unittest.TestCase): """ Check additional arguments passed to pre_commit_run are passed to pre-commit """ - args = self.run_script(MAIN, ["more", "arguments"]) + args = self.run_script("main", ["more", "arguments"]) expect = ["run", "more", "arguments", "--all-files"] self.assertListEqual(expect, args) @@ -189,14 +185,14 @@ class PreCommitRunTests(unittest.TestCase): http: "fatal: error processing shallow info: 4" """ args = self.run_script( - MAIN, + "main", CI_PIPELINE_SOURCE="merge_request_event", - CI_MERGE_REQUEST_TARGET_BRANCH_NAME=LCA, + CI_MERGE_REQUEST_TARGET_BRANCH_NAME="lca", ) expect = [ "run", - f"--from-ref={self.branches[LCA].head}", - f"--to-ref={self.repo.get_sha(MAIN)}", + f"--from-ref={self.repo.get_sha('lca')}", + f"--to-ref={self.repo.get_sha('main')}", ] self.assertListEqual(expect, args) -- GitLab From e27fb4417487f20c1a9c5924141a976e9fd63cfa Mon Sep 17 00:00:00 2001 From: Dom Sekotill Date: Sat, 9 Apr 2022 01:45:35 +0100 Subject: [PATCH 2/5] Pass pre_commit_run test refs as full refs --- .../unit/gitlab_ci/pre_commit_run/runner.bash | 22 ++++++++++++++----- tests/unit/gitlab_ci/pre_commit_run/tests.py | 14 ++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/tests/unit/gitlab_ci/pre_commit_run/runner.bash b/tests/unit/gitlab_ci/pre_commit_run/runner.bash index e0e85a3..cf88c36 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/runner.bash +++ b/tests/unit/gitlab_ci/pre_commit_run/runner.bash @@ -53,6 +53,10 @@ get_sha() { GIT_DIR=$1 git rev-parse --revs-only --verify $2 } +get_ref_name() { + sed 's@refs/[^/]\+/@@' <<<"$1" +} + get_head() { GIT_DIR=$1 git symbolic-ref HEAD | sed 's@refs/heads/@@' } @@ -60,7 +64,7 @@ get_head() { # Handle arguments cd "${1? The directory to run from, this should be the top of the git repository}" source "${2? The full path to the Bash file containing pre_commit_run}" -declare COMMIT_BRANCH=${3? Need the head reference of a target commmit} +declare COMMIT_REF=${3? Need the head or tag reference of a target commit} declare SOURCE_REPO=${4? Need the path to the source repository fixture} shift 4 @@ -76,18 +80,26 @@ declare -x CI_MERGE_REQUEST_TARGET_BRANCH_NAME # Set appropriate values for CI_* variables CI_REPOSITORY_URL=file://$SOURCE_REPO CI_DEFAULT_BRANCH=$(get_head $SOURCE_REPO) -CI_COMMIT_SHA=$(get_sha $SOURCE_REPO $COMMIT_BRANCH) +CI_COMMIT_SHA=$(get_sha $SOURCE_REPO $COMMIT_REF) case ${CI_PIPELINE_SOURCE:=push} in merge_request_event) assert '[[ ! -v CI_COMMIT_BEFORE_SHA ]]' \ "CI_COMMIT_BEFORE_SHA cannot be set for merge request pipelines" : ${CI_MERGE_REQUEST_TARGET_BRANCH_NAME:=$CI_DEFAULT_BRANCH} - CI_MERGE_REQUEST_SOURCE_BRANCH_NAME=$COMMIT_BRANCH + CI_MERGE_REQUEST_SOURCE_BRANCH_NAME=$(get_ref_name $COMMIT_REF) CI_COMMIT_BEFORE_SHA=$NULL_SHA ;; - *) - CI_COMMIT_BRANCH=$COMMIT_BRANCH + *) case $COMMIT_REF in + refs/heads/*) + CI_COMMIT_BRANCH=$(get_ref_name $COMMIT_REF) + ;; + refs/tags/*) + CI_PIPELINE_SOURCE=push + CI_COMMIT_TAG=$(get_ref_name $COMMIT_REF) + ;; + *) echo >&2 "Unrecognised reference type: $COMMIT_REF"; exit 2 ;; + esac ;; esac diff --git a/tests/unit/gitlab_ci/pre_commit_run/tests.py b/tests/unit/gitlab_ci/pre_commit_run/tests.py index 0adc4e5..6a0fc0d 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/tests.py +++ b/tests/unit/gitlab_ci/pre_commit_run/tests.py @@ -102,7 +102,7 @@ class PreCommitRunTests(unittest.TestCase): to_sha = self.repo.get_sha("main") args = self.run_script( - "main", + "refs/heads/main", CI_COMMIT_BEFORE_SHA=from_sha, ) @@ -118,7 +118,7 @@ class PreCommitRunTests(unittest.TestCase): Check that calling in merge request pipelines finds the LCA with the target branch """ args = self.run_script( - "fork", + "refs/heads/fork", CI_PIPELINE_SOURCE="merge_request_event", ) @@ -135,7 +135,7 @@ class PreCommitRunTests(unittest.TestCase): CI_COMMIT_BRANCH != CI_DEFAULT_BRANCH """ - args = self.run_script("fork") + args = self.run_script("refs/heads/fork") expect = [ "run", @@ -150,7 +150,7 @@ class PreCommitRunTests(unittest.TestCase): CI_COMMIT_BRANCH == CI_DEFAULT_BRANCH """ - args = self.run_script("main") + args = self.run_script("refs/heads/main") expect = ["run", "--all-files"] self.assertListEqual(expect, args) @@ -159,7 +159,7 @@ class PreCommitRunTests(unittest.TestCase): """ Check that pushing to a new orphan branch check all files """ - args = self.run_script("orphan") + args = self.run_script("refs/heads/orphan") expect = ["run", "--all-files"] self.assertListEqual(expect, args) @@ -168,7 +168,7 @@ class PreCommitRunTests(unittest.TestCase): """ Check additional arguments passed to pre_commit_run are passed to pre-commit """ - args = self.run_script("main", ["more", "arguments"]) + args = self.run_script("refs/heads/main", ["more", "arguments"]) expect = ["run", "more", "arguments", "--all-files"] self.assertListEqual(expect, args) @@ -185,7 +185,7 @@ class PreCommitRunTests(unittest.TestCase): http: "fatal: error processing shallow info: 4" """ args = self.run_script( - "main", + "refs/heads/main", CI_PIPELINE_SOURCE="merge_request_event", CI_MERGE_REQUEST_TARGET_BRANCH_NAME="lca", ) -- GitLab From 00dad5870f85e1c022fa5cb04c6e4969642565db Mon Sep 17 00:00:00 2001 From: Dom Sekotill Date: Sat, 9 Apr 2022 01:56:04 +0100 Subject: [PATCH 3/5] Remove unnecessary return in pre_commit_run tests --- tests/unit/gitlab_ci/pre_commit_run/fixtures.py | 4 +--- tests/unit/gitlab_ci/pre_commit_run/tests.py | 6 +----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/unit/gitlab_ci/pre_commit_run/fixtures.py b/tests/unit/gitlab_ci/pre_commit_run/fixtures.py index 933ec84..feb7438 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/fixtures.py +++ b/tests/unit/gitlab_ci/pre_commit_run/fixtures.py @@ -128,7 +128,7 @@ class Repo: return Sha(proc.stdout.decode().strip()) -def setup_fixture_repo(repo: Repo) -> dict[Ref, Branch]: +def setup_fixture_repo(repo: Repo) -> None: r""" Create a sample git repository as a test fixture @@ -157,8 +157,6 @@ def setup_fixture_repo(repo: Repo) -> dict[Ref, Branch]: orphan = Branch(repo, "orphan", create_commit(repo)) orphan.head = create_commit_path(repo, orphan.root, 5) - return {b.name: b for b in (main, fork, lca, orphan)} - def create_commit_path(repo: Repo, source: Sha, number: int) -> Sha: """ diff --git a/tests/unit/gitlab_ci/pre_commit_run/tests.py b/tests/unit/gitlab_ci/pre_commit_run/tests.py index 6a0fc0d..6fa05f2 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/tests.py +++ b/tests/unit/gitlab_ci/pre_commit_run/tests.py @@ -22,10 +22,7 @@ import subprocess import unittest from pathlib import Path from typing import Iterable -from typing import Mapping -from .fixtures import Branch -from .fixtures import Ref from .fixtures import Repo from .fixtures import setup_fixture_repo @@ -43,7 +40,6 @@ class PreCommitRunTests(unittest.TestCase): repo: Repo test_repo: Repo - branches: Mapping[Ref, Branch] @classmethod def setUpClass(cls) -> None: @@ -51,7 +47,7 @@ class PreCommitRunTests(unittest.TestCase): Prepare fixtures for the entire test case """ cls.repo = Repo(init=True) - cls.branches = setup_fixture_repo(cls.repo) + setup_fixture_repo(cls.repo) @classmethod def tearDownClass(cls) -> None: -- GitLab From fad15cf24801b50048da23cb55795e2ac0b417d0 Mon Sep 17 00:00:00 2001 From: Dom Sekotill Date: Sat, 9 Apr 2022 01:56:47 +0100 Subject: [PATCH 4/5] Add a tag test to pre_commit_run tests --- tests/unit/gitlab_ci/pre_commit_run/fixtures.py | 17 ++++++++++------- tests/unit/gitlab_ci/pre_commit_run/tests.py | 9 +++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/unit/gitlab_ci/pre_commit_run/fixtures.py b/tests/unit/gitlab_ci/pre_commit_run/fixtures.py index feb7438..1b85635 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/fixtures.py +++ b/tests/unit/gitlab_ci/pre_commit_run/fixtures.py @@ -135,13 +135,13 @@ def setup_fixture_repo(repo: Repo) -> None: The repository consists of three branches, two with common ancestors and one with no commonality with the other two: - ┌──────────────────────────────────┐ - │ "lca" (Last common ancestor) │ - │ ↓ │ - │ ○─○─○─○─○─○─○─○─○─● "main" │ - │ └─○─○─○─○─● "fork" │ - │ ○─○─○─○─○─○─○─○─○─● "orphan" │ - └──────────────────────────────────┘ + ┌──────────────────────────────────────────┐ + │ "lca" (Last common ancestor) │ + │ ↓ │ + │ ○─○─○─○─○─○─○─○─○─● "main", tag:"test" │ + │ └─○─○─○─○─● "fork" │ + │ ○─○─○─○─○─○─○─○─○─● "orphan" │ + └──────────────────────────────────────────┘ """ random.seed(1) @@ -157,6 +157,9 @@ def setup_fixture_repo(repo: Repo) -> None: orphan = Branch(repo, "orphan", create_commit(repo)) orphan.head = create_commit_path(repo, orphan.root, 5) + # Make test tag + repo.command("tag", "test", main.head) + def create_commit_path(repo: Repo, source: Sha, number: int) -> Sha: """ diff --git a/tests/unit/gitlab_ci/pre_commit_run/tests.py b/tests/unit/gitlab_ci/pre_commit_run/tests.py index 6fa05f2..f4991fb 100644 --- a/tests/unit/gitlab_ci/pre_commit_run/tests.py +++ b/tests/unit/gitlab_ci/pre_commit_run/tests.py @@ -160,6 +160,15 @@ class PreCommitRunTests(unittest.TestCase): expect = ["run", "--all-files"] self.assertListEqual(expect, args) + def test_tag(self) -> None: + """ + Check that pushing a tag works like pushing to a new branch + """ + args = self.run_script("refs/tags/test") + + expect = ["run", "--all-files"] + self.assertListEqual(expect, args) + def test_additional_arguments(self) -> None: """ Check additional arguments passed to pre_commit_run are passed to pre-commit -- GitLab From 1e17375be2a3149a51abd47c4b3886753f2044d6 Mon Sep 17 00:00:00 2001 From: Dom Sekotill Date: Sat, 9 Apr 2022 02:01:21 +0100 Subject: [PATCH 5/5] Fix pre_commit_run for tags --- .gitlab-ci.pre-commit-run.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.pre-commit-run.bash b/.gitlab-ci.pre-commit-run.bash index 704e716..54d829b 100644 --- a/.gitlab-ci.pre-commit-run.bash +++ b/.gitlab-ci.pre-commit-run.bash @@ -44,7 +44,7 @@ pre_commit_run() ( fetch_ref $CI_COMMIT_BEFORE_SHA elif [[ -v CI_MERGE_REQUEST_TARGET_BRANCH_NAME ]]; then find_lca $CI_MERGE_REQUEST_SOURCE_BRANCH_NAME $CI_MERGE_REQUEST_TARGET_BRANCH_NAME - elif [[ $CI_COMMIT_BRANCH != $CI_DEFAULT_BRANCH ]]; then + elif [[ -v CI_COMMIT_BRANCH ]] && [[ $CI_COMMIT_BRANCH != $CI_DEFAULT_BRANCH ]]; then find_lca $CI_COMMIT_BRANCH $CI_DEFAULT_BRANCH fi -- GitLab