From 37f0f4ae54b51432e4d3ad242e5b01553d6b59ad Mon Sep 17 00:00:00 2001
From: Jordan Torbiak <torbiak@gmail.com>
Date: Fri, 13 Oct 2023 14:40:14 -0600
Subject: [PATCH] Break the circular dependency between Util and Repo

The high-level test functions test_autofixup() and
test_autofixup_strict() didn't really fit in Util.
---
 t/autofixup.t |  56 ++++++++++++------------
 t/test.pl     | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++
 t/util.pl     | 107 ---------------------------------------------
 3 files changed, 147 insertions(+), 135 deletions(-)
 create mode 100644 t/test.pl

diff --git a/t/autofixup.t b/t/autofixup.t
index 6e2daf3..1138067 100755
--- a/t/autofixup.t
+++ b/t/autofixup.t
@@ -5,13 +5,13 @@ use warnings FATAL => 'all';
 use English qw(-no_match_vars);
 use Test::More;
 
+require './t/test.pl';
 require './t/util.pl';
-require './git-autofixup';
 
 Util::check_test_deps();
 plan tests => 43;
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "single-line change gets autofixed",
     strict => [0..2],
     topic_commits => [{a => "a1\n"}],
@@ -30,7 +30,7 @@ index da0f8ed..c1827f0 100644
 EOF
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "adjacent change gets autofixed",
     strict => [0..1],
     upstream_commits => [{a => "a3\n"}],
@@ -51,7 +51,7 @@ index 76642d4..2cdcdb0 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "adjacent change doesn't get autofixed if strict=2",
     upstream_commits => [{a => "a3\n"}],
     topic_commits => [{a => "a1\na3\n"}],
@@ -61,7 +61,7 @@ Util::test_autofixup(
     exit_code => 2,
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => 'fixups are created for additions surrounded by topic commit lines when strict=2',
     topic_commits => [{a => "a1\na3\n", b => "b1\n", c => "c2\n"}],
     unstaged => {a => "a1\na2\na3\n", b => "b1\nb2\n", c => "c1\nc2\n"},
@@ -95,7 +95,7 @@ index 16f9ec0..d0aaf97 100644
 EOF
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "removed file doesn't get autofixed",
     strict => [0..2],
     topic_commits => [sub { Util::write_file(a => "a1\n"); }],
@@ -104,7 +104,7 @@ Util::test_autofixup_strict(
     log_want => '',
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "re-added file doesn't get autofixed",
     strict => [0..2],
     topic_commits => [
@@ -116,7 +116,7 @@ Util::test_autofixup_strict(
     log_want => '',
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "re-added line gets autofixed into the commit blamed for the adjacent context",
     # During rebase the line will just get removed again by the next commit.
     # --strict can be used to avoid creating a fixup in this case, where the
@@ -145,7 +145,7 @@ index da0f8ed..0016606 100644
 EOF
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "removed lines get autofixed",
     strict => [0..2],
     topic_commits => [{a => "a1\n", b => "b1\nb2\n"}],
@@ -170,7 +170,7 @@ index 9b89cd5..e6bfff5 100644
 EOF
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => 'no fixups are created for upstream commits',
     strict => [0..2],
     upstream_commits => [{a => "a1\n"}],
@@ -179,7 +179,7 @@ Util::test_autofixup_strict(
     log_want => '',
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => 'fixups are created for hunks changing lines blamed by upstream if strict=0',
     # This depends on the number of context lines kept when creating diffs. git
     # keeps 3 by default.
@@ -203,7 +203,7 @@ index 125d560..cc1aa32 100644
 +a3b
 EOF
 );
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => 'no fixups are created for hunks changing lines blamed by upstream if strict > 0',
     # This depends on the number of context lines kept when creating diffs. git
     # keeps 3 by default.
@@ -215,7 +215,7 @@ Util::test_autofixup_strict(
     log_want => '',
 );
 
-Util::test_autofixup_strict(
+Test::autofixup_strict(
     name => "hunks blamed on a fixup! commit are assigned to that fixup's target",
     strict => [0..2],
     topic_commits => [
@@ -240,7 +240,7 @@ index c1827f0..b792f74 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "removed line gets autofixed when context=0",
     topic_commits => [{a => "a1\na2\n"}],
     unstaged => {a => "a1\n"},
@@ -259,7 +259,7 @@ index 0016606..da0f8ed 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "added line is ignored when context=0",
     topic_commits => [{a => "a1\n"}],
     unstaged => {a => "a1\na2\n"},
@@ -268,7 +268,7 @@ Util::test_autofixup(
     log_want => '',
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "ADJACENCY assignment is used as a fallback for multiple context targets",
     topic_commits => [
         {a => "a1\n"},
@@ -290,7 +290,7 @@ index 0016606..a0ef52c 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "Works when run in a subdir of the repo root",
     topic_commits => [
         sub {
@@ -314,7 +314,7 @@ index da0f8ed..0016606 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "file without newline at EOF gets autofixed",
     topic_commits => [{a => "a1\na2"}],
     unstaged => {'a' => "a1\na2\n"},
@@ -334,7 +334,7 @@ index c928c51..0016606 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "multiple hunks in the same file get autofixed",
     topic_commits => [
         {a => "a1.0\na2\na3\na4\na5\na6\na7\na8\na9.0\n"},
@@ -371,7 +371,7 @@ index 50de7e8..d9f44da 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "single-line change gets autofixed when mnemonic prefixes are enabled",
     topic_commits => [{a => "a1\n"}],
     unstaged => {a => "a2\n"},
@@ -390,7 +390,7 @@ index da0f8ed..c1827f0 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "single-line change gets autofixed when diff.external is set",
     topic_commits => [{a => "a1\n"}],
     unstaged => {a => "a2\n"},
@@ -409,7 +409,7 @@ index da0f8ed..c1827f0 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => 'exit code is 1 when some hunks are assigned',
     upstream_commits => [{a => "a1\n"}],
     topic_commits => [{b => "b1\n"}],
@@ -428,7 +428,7 @@ index c9c6af7..e6bfff5 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "multiple hunks to the same commit get autofixed",
     topic_commits => [
         {a => "a1.0\na2\na3\na4\na5\na6\na7\na8\na9.0\n"},
@@ -465,7 +465,7 @@ index 5d11004..0054137 100644
 +a9.1
 });
 
-Util::test_autofixup(
+Test::autofixup(
     name => "only staged hunks get autofixed",
     topic_commits => [{a => "a1\n", b => "b1\n"}],
     staged => {a => "a2\n"},
@@ -493,7 +493,7 @@ index c9c6af7..e6bfff5 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "staged hunks that aren't autofixed remain in index",
     upstream_commits => [{b => "b1\n"}],
     topic_commits => [{a => "a1\n", , c => "c1\n"}],
@@ -538,7 +538,7 @@ index c9c6af7..e6bfff5 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "filename with spaces",
     topic_commits => [{"filename with spaces" => "a1\n"}],
     unstaged => {"filename with spaces" => "a2\n"},
@@ -556,7 +556,7 @@ index da0f8ed..c1827f0 100644
 EOF
 );
 
-Util::test_autofixup(
+Test::autofixup(
     name => "filename with unusual characters",
     topic_commits => [{"ff\f nak\025 dq\" fei飞.txt" => "a1\n"}],
     unstaged => {"ff\f nak\025 dq\" fei飞.txt" => "a2\n"},
@@ -576,7 +576,7 @@ EOF
 
 SKIP: {
     skip "can't put backslashes in filenames on windows" if $OSNAME eq 'cygwin';
-    Util::test_autofixup(
+    Test::autofixup(
         name => "filename with backslash",
         topic_commits => [{"hack\\.txt" => "a1\n"}],
         unstaged => {"hack\\.txt" => "a2\n"},
diff --git a/t/test.pl b/t/test.pl
new file mode 100644
index 0000000..614cee4
--- /dev/null
+++ b/t/test.pl
@@ -0,0 +1,119 @@
+# Long test functions that could conceivably be used in multiple .t files.
+
+package Test;
+
+use strict;
+use warnings FATAL => 'all';
+
+use Carp qw(croak);
+use Cwd;
+use Test::More;
+
+require './t/util.pl';
+require './t/repo.pl';
+
+# Run autofixup() with each of the given strictness levels.
+sub autofixup_strict {
+    my %args = @_;
+    my $strict_levels = $args{strict} or croak "strictness levels not given";
+    delete $args{strict};
+    my $autofixup_opts = $args{autofixup_opts} || [];
+    if (grep /^(--strict|-s)/, @{$autofixup_opts}) {
+        croak "strict option already given";
+    }
+    my $name = $args{name} || croak "name not given";
+    for my $strict (@{$strict_levels}) {
+        $args{name} = "$name, strict=$strict";
+        $args{autofixup_opts} = ['-s' => $strict, @{$autofixup_opts}];
+        autofixup(%args);
+    }
+}
+
+# autofixup initializes a git repo in a tempdir, creates given "upstream" and
+# "topic" commits, applies changes to the working directory, runs autofixup,
+# and compares wanted `git log` and `git diff` outputs to actual ones.
+#
+# Arguments are given as a hash:
+# name: test name or description
+# upstream_commits: sub or hash refs that must not be fixed up
+# topic_commits: sub or hash refs representing commits that can be fixed up
+# unstaged: sub or hashref of working directory changes
+# staged: sub or hashref of index changes
+# log_want: expected log output for new fixup commited
+# staged_want: expected log output for the staging area
+# unstaged_want: expected diff output for the working tree
+# autofixup_opts: command-line options to pass thru to autofixup
+# git_config: hashref of git config key/value pairs
+#
+# The upstream_commits and topic_commits arguments are heterogeneous lists of
+# sub and hash refs. Hash refs are interpreted as being maps of filenames to
+# contents to be written. If more flexibility is needed a subref can be given
+# to manipulate the working directory.
+sub autofixup {
+    my %args = @_;
+    my $name = defined($args{name}) ? $args{name}
+             : croak "no test name given";
+    my $upstream_commits = $args{upstream_commits} || [];
+    my $topic_commits = $args{topic_commits} || [];
+    my $unstaged = defined($args{unstaged}) ? $args{unstaged}
+                 : croak "no unstaged changes given";
+    my $staged = $args{staged};
+    my $log_want = defined($args{log_want}) ? $args{log_want}
+                 : croak "wanted log output not given";
+    my $staged_want = $args{staged_want};
+    my $unstaged_want = $args{unstaged_want};
+    my $exit_code_want = $args{exit_code};
+    my $autofixup_opts = $args{autofixup_opts} || [];
+    push @{$autofixup_opts}, '--exit-code';
+    my $git_config = $args{git_config} || {};
+    if (!$upstream_commits && !$topic_commits) {
+        croak "no upstream or topic commits given";
+    }
+    if (exists $args{strict}) {
+        croak "strict key given; use test_autofixup_strict instead";
+    }
+
+    local $ENV{GIT_CONFIG_COUNT} = scalar keys %$git_config;
+    my $git_config_env = Util::git_config_env_vars($git_config);
+    local (@ENV{keys %$git_config_env});
+    for my $k (keys %$git_config_env) {
+        $ENV{$k} = $git_config_env->{$k};
+    }
+
+    eval {
+        my $repo = Repo->new();
+
+        $repo->create_commits(@$upstream_commits);
+        my $upstream_rev = $repo->current_commit_sha();
+
+        $repo->create_commits(@$topic_commits);
+        my $pre_fixup_rev = $repo->current_commit_sha();
+
+        if (defined($staged)) {
+            $repo->write_change($staged);
+            # We're at the repo root, so using -A will change everything even
+            # in pre-v2 versions of git. See git commit 808d3d717e8.
+            Util::run(qw(git add -A));
+        }
+
+        $repo->write_change($unstaged);
+
+        Util::run('git', '--no-pager',  'log', "--format='%h %s'", "${upstream_rev}..");
+        my $exit_code_got = $repo->autofixup(@{$autofixup_opts}, $upstream_rev);
+
+        my $ok = Util::exit_code_ok(want => $exit_code_want, got => $exit_code_got);
+        my $wants = {
+            fixup_log => $log_want,
+            staged => $staged_want,
+            unstaged => $unstaged_want,
+        };
+        $ok &&= Util::repo_state_ok($repo, $pre_fixup_rev, $wants);
+        ok($ok, $name);
+    };
+    if ($@) {
+        diag($@);
+        fail($name);
+    }
+    return;
+}
+
diff --git a/t/util.pl b/t/util.pl
index 9433e72..c49a6b6 100644
--- a/t/util.pl
+++ b/t/util.pl
@@ -1,4 +1,3 @@
-#!/usr/bin/perl
 package Util;
 
 use strict;
@@ -38,112 +37,6 @@ sub is_git_for_windows {
     return $version =~ /\.(?:windows|msysgit)\./i;
 }
 
-# Run test_autofixup() with each of the given strictness levels.
-sub test_autofixup_strict {
-    my %args = @_;
-    my $strict_levels = $args{strict} or croak "strictness levels not given";
-    delete $args{strict};
-    my $autofixup_opts = $args{autofixup_opts} || [];
-    if (grep /^(--strict|-s)/, @{$autofixup_opts}) {
-        croak "strict option already given";
-    }
-    my $name = $args{name} || croak "name not given";
-    for my $strict (@{$strict_levels}) {
-        $args{name} = "$name, strict=$strict";
-        $args{autofixup_opts} = ['-s' => $strict, @{$autofixup_opts}];
-        test_autofixup(%args);
-    }
-}
-
-# test_autofixup initializes a git repo in a tempdir, creates given "upstream"
-# and "topic" commits, applies changes to the working directory, runs
-# autofixup, and compares wanted `git log` and `git diff` outputs to actual
-# ones.
-#
-# Arguments are given as a hash:
-# name: test name or description
-# upstream_commits: sub or hash refs that must not be fixed up
-# topic_commits: sub or hash refs representing commits that can be fixed up
-# unstaged: sub or hashref of working directory changes
-# staged: sub or hashref of index changes
-# log_want: expected log output for new fixup commited
-# staged_want: expected log output for the staging area
-# unstaged_want: expected diff output for the working tree
-# autofixup_opts: command-line options to pass thru to autofixup
-# git_config: hashref of git config key/value pairs
-#
-# The upstream_commits and topic_commits arguments are heterogeneous lists of
-# sub and hash refs. Hash refs are interpreted as being maps of filenames to
-# contents to be written. If more flexibility is needed a subref can be given
-# to manipulate the working directory.
-sub test_autofixup {
-    my %args = @_;
-    my $name = defined($args{name}) ? $args{name}
-             : croak "no test name given";
-    my $upstream_commits = $args{upstream_commits} || [];
-    my $topic_commits = $args{topic_commits} || [];
-    my $unstaged = defined($args{unstaged}) ? $args{unstaged}
-                 : croak "no unstaged changes given";
-    my $staged = $args{staged};
-    my $log_want = defined($args{log_want}) ? $args{log_want}
-                 : croak "wanted log output not given";
-    my $staged_want = $args{staged_want};
-    my $unstaged_want = $args{unstaged_want};
-    my $exit_code_want = $args{exit_code};
-    my $autofixup_opts = $args{autofixup_opts} || [];
-    push @{$autofixup_opts}, '--exit-code';
-    my $git_config = $args{git_config} || {};
-    if (!$upstream_commits && !$topic_commits) {
-        croak "no upstream or topic commits given";
-    }
-    if (exists $args{strict}) {
-        croak "strict key given; use test_autofixup_strict instead";
-    }
-
-    local $ENV{GIT_CONFIG_COUNT} = scalar keys %$git_config;
-    my $git_config_env = git_config_env_vars($git_config);
-    local (@ENV{keys %$git_config_env});
-    for my $k (keys %$git_config_env) {
-        $ENV{$k} = $git_config_env->{$k};
-    }
-
-    eval {
-        my $repo = Repo->new();
-
-        $repo->create_commits(@$upstream_commits);
-        my $upstream_rev = $repo->current_commit_sha();
-
-        $repo->create_commits(@$topic_commits);
-        my $pre_fixup_rev = $repo->current_commit_sha();
-
-        if (defined($staged)) {
-            $repo->write_change($staged);
-            # We're at the repo root, so using -A will change everything even
-            # in pre-v2 versions of git. See git commit 808d3d717e8.
-            run(qw(git add -A));
-        }
-
-        $repo->write_change($unstaged);
-
-        run('git', '--no-pager',  'log', "--format='%h %s'", "${upstream_rev}..");
-        my $exit_code_got = $repo->autofixup(@{$autofixup_opts}, $upstream_rev);
-
-        my $ok = exit_code_ok(want => $exit_code_want, got => $exit_code_got);
-        my $wants = {
-            fixup_log => $log_want,
-            staged => $staged_want,
-            unstaged => $unstaged_want,
-        };
-        $ok &&= repo_state_ok($repo, $pre_fixup_rev, $wants);
-        ok($ok, $name);
-    };
-    if ($@) {
-        diag($@);
-        fail($name);
-    }
-    return;
-}
-
 # Convert a hashref of git config key-value pairs to a hashref of
 # GIT_CONFIG_{KEY,VALUE}_<i> pairs suitable for setting as environment
 # variables.