From 215f53133e5879ff35cba142bd5c643a7ccd4249 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 21 Aug 2023 15:35:04 +0200 Subject: repository: Fix ReplicateRepository test depending on host umask One of our tests for ReplicateRepository implicitly depends on the host umask. This is becaues we have a hardcoded vote that expects a specific vote for the hooks vote, but that vote does in fact include permission bits that change depending on the current umask. Fix this by dynamically detecting the expecte mode bits such that we can compute the expected vote instead of having to hardcode it. --- internal/gitaly/service/repository/replicate_test.go | 11 ++++++++++- internal/testhelper/testhelper.go | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index 7b678ec9d..b5b542da5 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -3,6 +3,7 @@ package repository import ( "bytes" "context" + "encoding/binary" "fmt" "io" "io/fs" @@ -685,6 +686,8 @@ func TestReplicateRepository_transactional(t *testing.T) { } func testReplicateRepositoryTransactional(t *testing.T, ctx context.Context) { + t.Parallel() + cfgBuilder := testcfg.NewGitalyCfgBuilder(testcfg.WithStorages("default", "replica")) cfg := cfgBuilder.Build(t) @@ -743,7 +746,13 @@ func testReplicateRepositoryTransactional(t *testing.T, ctx context.Context) { // There is a gitconfig though, so the vote should reflect its contents. gitconfigVote := voting.VoteFromData(testhelper.MustReadFile(t, filepath.Join(sourceRepoPath, "config"))) - noHooksVote := voting.Vote("fd69c38637bf443296edc19e2b2c649d0502f7c0") + // The custom hooks directory is empty, so we will only write a single empty directory. Thus, we only + // write the relative path of that directory (".") as well as its mode bits to the vote hash. Note that + // we use a temporary file here to figure out the expected permissions as they would in fact be subject + // to change depending on the current umask. + noHooksVoteData := [5]byte{'.', 0, 0, 0, 0} + binary.BigEndian.PutUint32(noHooksVoteData[1:], uint32(fs.ModeDir|testhelper.MaskedPerms(t, perm.PublicDir))) + noHooksVote := voting.VoteFromData(noHooksVoteData[:]) expectedVotes := []voting.Vote{ // We cannot easily derive these first two votes: they are based on the complete diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 03869008d..24481581c 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "math/rand" "net" "net/http" @@ -328,6 +329,22 @@ func WriteExecutable(tb testing.TB, path string, content []byte) string { return path } +// MaskedPerms determines permission bits with the current process umask applied. This is preferable over calls to +// `perm.GetUmask()` because it can be executed in parallel. Note that this function does not include non-permission +// mode bits like `fs.ModeDir` or `fs.ModeSocket`. You can OR these manually as required. +func MaskedPerms(tb testing.TB, mode fs.FileMode) fs.FileMode { + tb.Helper() + + f, err := os.OpenFile(filepath.Join(TempDir(tb), "file"), os.O_CREATE|os.O_EXCL, mode) + require.NoError(tb, err) + defer MustClose(tb, f) + + stat, err := f.Stat() + require.NoError(tb, err) + + return stat.Mode().Perm() +} + // Unsetenv unsets an environment variable. The variable will be restored after the test has // finished. func Unsetenv(tb testing.TB, key string) { -- cgit v1.2.3