Merge branch 'pks-operations-convert-test-hook-to-bash' into 'master'

operations: Convert test hooks to use Bash instead of Ruby

See merge request gitlab-org/gitaly!4803

2 files changed, 58 insertions, 74 deletions

diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index 798772597..f223e039e 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -5,7 +5,6 @@ package main
 import (
 	"bytes"
 	"context"
-	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
@@ -27,6 +26,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/hook"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitlab"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/text"
 	gitalylog "gitlab.com/gitlab-org/gitaly/v15/internal/log"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
@@ -273,30 +273,27 @@ func TestHooksUpdate(t *testing.T) {
 }
 
 func testHooksUpdate(t *testing.T, ctx context.Context, cfg config.Cfg, glValues glHookValues) {
-	repo, repoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
+	repo, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
 
 	refval, oldval, newval := "refval", strings.Repeat("a", 40), strings.Repeat("b", 40)
 
+	// Write a custom update hook that dumps all arguments seen by the hook...
+	customHookArgsPath := filepath.Join(testhelper.TempDir(t), "containsarguments")
+	testhelper.WriteExecutable(t,
+		filepath.Join(repoPath, "custom_hooks", "update.d", "dumpargsscript"),
+		[]byte(fmt.Sprintf(`#!/bin/bash
+			echo "$@" >%q
+		`, customHookArgsPath)),
+	)
+
+	// ... and a second custom hook that dumps the environment variables.
+	customHookEnvPath := gittest.WriteEnvToCustomHook(t, repoPath, "update")
+
+	var stdout, stderr bytes.Buffer
 	cmd := exec.Command(cfg.BinaryPath("gitaly-hooks"))
 	cmd.Args = []string{"update", refval, oldval, newval}
 	cmd.Env = envForHooks(t, ctx, cfg, repo, glValues, proxyValues{})
 	cmd.Dir = repoPath
-
-	tempDir := testhelper.TempDir(t)
-
-	customHookArgsPath := filepath.Join(tempDir, "containsarguments")
-	dumpArgsToTempfileScript := fmt.Sprintf(`#!/usr/bin/env ruby
-require 'json'
-open('%s', 'w') { |f| f.puts(JSON.dump(ARGV)) }
-`, customHookArgsPath)
-	// write a custom hook to path/to/repo.git/custom_hooks/update.d/dumpargsscript which dumps the args into a tempfile
-	testhelper.WriteExecutable(t, filepath.Join(repoPath, "custom_hooks", "update.d", "dumpargsscript"), []byte(dumpArgsToTempfileScript))
-
-	// write a custom hook to path/to/repo.git/custom_hooks/update which dumps the env into a tempfile
-	customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "update")
-
-	var stdout, stderr bytes.Buffer
-
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
 	cmd.Dir = repoPath
@@ -305,15 +302,14 @@ open('%s', 'w') { |f| f.puts(JSON.dump(ARGV)) }
 	require.Empty(t, stdout.String())
 	require.Empty(t, stderr.String())
 
-	require.FileExists(t, customHookArgsPath)
-
-	var inputs []string
-
-	b := testhelper.MustReadFile(t, customHookArgsPath)
-	require.NoError(t, json.Unmarshal(b, &inputs))
-	require.Equal(t, []string{refval, oldval, newval}, inputs)
+	// Ensure that the hook was executed with the expected arguments...
+	require.Equal(t,
+		fmt.Sprintf("%s %s %s", refval, oldval, newval),
+		text.ChompBytes(testhelper.MustReadFile(t, customHookArgsPath)),
+	)
 
-	output := string(testhelper.MustReadFile(t, customHookOutputPath))
+	// ... and with the expected environment variables.
+	output := string(testhelper.MustReadFile(t, customHookEnvPath))
 	require.Contains(t, output, "GL_USERNAME="+glValues.GLUsername)
 	require.Contains(t, output, "GL_ID="+glValues.GLID)
 	require.Contains(t, output, "GL_REPOSITORY="+repo.GetGlRepository())
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index c7b1f6ea7..37fa8af95 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -85,60 +85,48 @@ func TestUserDeleteTag_hooks(t *testing.T) {
 func writeAssertObjectTypePreReceiveHook(t *testing.T, repoPath, expectedObjectType string) {
 	t.Helper()
 
-	gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(
-		`#!/usr/bin/env ruby
-
-		# We match a non-ASCII ref_name below
-		Encoding.default_external = Encoding::UTF_8
-		Encoding.default_internal = Encoding::UTF_8
-
-		expected_object_type = %q
-		commands = STDIN.each_line.map(&:chomp)
-		unless commands.size == 1
-		abort "expected 1 ref update command, got #{commands.size}"
-		end
-
-		old_value, new_value, ref_name = commands[0].split(' ', 3)
-		abort 'missing new_value' unless new_value
-
-		out = IO.popen(%%W[git cat-file -t #{new_value}], &:read)
-		abort 'cat-file failed' unless $?.success?
-
-		if ref_name =~ /^refs\/[^\/]+\/skip-type-check-/
-		exit 0
-		end
-
-		unless out.chomp == expected_object_type
-		abort "pre-receive hook error: expected '#{ref_name}' update of '#{old_value}' (a) -> '#{new_value}' (b) for 'b' to be a '#{expected_object_type}' object, got '#{out}'"
-		end
+	gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(`#!/bin/bash
+		i=0
+		while read oldvalue newvalue reference
+		do
+			i=$((i+1))
+
+			if [[ "${reference}" =~ skip-type-check- ]]
+			then
+				continue
+			fi
+
+			type="$(git cat-file -t "${newvalue}")"
+			if test "%[1]s" != "${type}"
+			then
+				echo "expected %[1]s, got ${type}" >&2
+				exit 1
+			fi
+		done
+
+		if test "$i" -ne 1
+		then
+			echo "expected exactly one reference update, got ${i}" >&2
+			exit 1
+		fi
 	`, expectedObjectType)))
 }
 
 func writeAssertObjectTypeUpdateHook(t *testing.T, repoPath, expectedObjectType string) {
 	t.Helper()
 
-	gittest.WriteCustomHook(t, repoPath, "update", []byte(fmt.Sprintf(
-		`#!/usr/bin/env ruby
-
-		# We match a non-ASCII ref_name below
-		Encoding.default_external = Encoding::UTF_8
-		Encoding.default_internal = Encoding::UTF_8
-
-		expected_object_type = %q
-		ref_name, old_value, new_value = ARGV[0..2]
-
-		abort "missing new_value" unless new_value
-
-		out = IO.popen(%%W[git cat-file -t #{new_value}], &:read)
-		abort 'cat-file failed' unless $?.success?
-
-		if ref_name =~ /^refs\/[^\/]+\/skip-type-check-/
-		exit 0
-		end
-
-		unless out.chomp == expected_object_type
-		abort "update hook error: expected '#{ref_name}' update of '#{old_value}' (a) -> '#{new_value}' (b) for 'b' to be a '#{expected_object_type}' object, got '#{out}'"
-		end
+	gittest.WriteCustomHook(t, repoPath, "update", []byte(fmt.Sprintf(`#!/bin/bash
+		if [[ "$1" =~ skip-type-check- ]]
+		then
+			exit 0
+		fi
+
+		type="$(git cat-file -t "$3")"
+		if test "%[1]s" != "${type}"
+		then
+			echo "expected %[1]s, got ${type}" >&2
+			exit 1
+		fi
 	`, expectedObjectType)))
 }