Implement final review comments from @rymai.25301-git-2.11-force-push-bug

- `raise "string"` raises a `RuntimeError` - no need to be explicit
- Remove top-level comment in the `RevList` class
- Use `%w()` instead of `%w[]`
- Extract an `environment_variables` method to cache `env.slice(*ALLOWED_VARIABLES)`
- Use `start_with?` for env variable validation instead of regex match
- Validation specs for each allowed environment variable were identical. Build them dynamically.
- Minor change to `popen3` expectation.

2 files changed, 9 insertions, 7 deletions

diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb
index e1c967a1f89..de0c9049ebf 100644
--- a/lib/gitlab/checks/force_push.rb
+++ b/lib/gitlab/checks/force_push.rb
@@ -13,7 +13,7 @@ module Gitlab
           if exit_status == 0
             missed_ref.present?
           else
-            raise RuntimeError, "Got a non-zero exit code while calling out to `git rev-list` in the force-push check."
+            raise "Got a non-zero exit code while calling out to `git rev-list` in the force-push check."
           end
         end
       end
diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb
index ecd038e04df..25e9d619697 100644
--- a/lib/gitlab/git/rev_list.rb
+++ b/lib/gitlab/git/rev_list.rb
@@ -1,11 +1,9 @@
-# Call out to the `git rev-list` command
-
 module Gitlab
   module Git
     class RevList
       attr_reader :project, :env
 
-      ALLOWED_VARIABLES = %w(GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES).freeze
+      ALLOWED_VARIABLES = %w[GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES].freeze
 
       def initialize(oldrev, newrev, project:, env: nil)
         @project = project
@@ -23,8 +21,8 @@ module Gitlab
       end
 
       def valid?
-        env.slice(*ALLOWED_VARIABLES).all? do |(name, value)|
-          value =~ /^#{project.repository.path_to_repo}/
+        environment_variables.all? do |(name, value)|
+          value.start_with?(project.repository.path_to_repo)
         end
       end
 
@@ -33,7 +31,11 @@ module Gitlab
       def parse_environment_variables
         return {} unless valid?
 
-        env.slice(*ALLOWED_VARIABLES)
+        environment_variables
+      end
+
+      def environment_variables
+        @environment_variables ||= env.slice(*ALLOWED_VARIABLES)
       end
     end
   end