Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit e47a933

Browse files
authored
gqltest: Ensure fusion test is actually cloning depot (#44539)
The previous tests we not testing cloning via p4-fusion, the following was happening: git p4 test runs, completes and deletes external service which then soft deletes repo. p4-fusion can't find repo to delete since it's name has changed so does nothing. Repo is still on disk so we end up fetching rather than cloning. Even if that fails, we still find the repo on disk and mark the test as passed. To fix this we now: * Delete the repo on disk before deleting the service. (Confirmed locally that the repo is removed on disk) * Clone a different repo in each test The tests were also refactored to make things a little more explicit.
1 parent fb922dd commit e47a933

File tree

2 files changed

+63
-37
lines changed

2 files changed

+63
-37
lines changed

dev/gqltest/external_service_test.go

Lines changed: 58 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
"github.com/google/go-cmp/cmp"
9+
"github.com/stretchr/testify/assert"
910

1011
"github.com/sourcegraph/sourcegraph/internal/extsvc"
1112
"github.com/sourcegraph/sourcegraph/internal/gqltestutil"
@@ -161,39 +162,49 @@ func TestExternalService_BitbucketServer(t *testing.T) {
161162
}
162163

163164
func TestExternalService_Perforce(t *testing.T) {
164-
const repoName = "perforce/test-perms"
165-
166-
run := func(t *testing.T, useFusion bool) {
167-
checkPerforceEnvironment(t)
168-
// Make sure the repo wasn't already cloned by another test
169-
if err := client.DeleteRepoFromDiskByName(repoName); err != nil {
170-
t.Fatal(err)
171-
}
172-
createPerforceExternalService(t, useFusion)
173-
174-
err := client.WaitForReposToBeCloned(repoName)
175-
if err != nil {
176-
t.Fatal(err)
177-
}
178-
179-
blob, err := client.GitBlob(repoName, "master", "README.md")
180-
if err != nil {
181-
t.Fatal(err)
182-
}
183-
184-
wantBlob := `This depot is used to test user and group permissions.
185-
`
186-
if diff := cmp.Diff(wantBlob, blob); diff != "" {
187-
t.Fatalf("Blob mismatch (-want +got):\n%s", diff)
188-
}
165+
for _, tc := range []struct {
166+
name string
167+
depot string
168+
useFusion bool
169+
blobPath string
170+
wantBlob string
171+
}{
172+
{
173+
name: "git p4",
174+
depot: "test-perms",
175+
useFusion: false,
176+
blobPath: "README.md",
177+
wantBlob: `This depot is used to test user and group permissions.
178+
`,
179+
},
180+
{
181+
name: "p4 fusion",
182+
depot: "integration-test-depot",
183+
useFusion: true,
184+
blobPath: "path.txt",
185+
wantBlob: `./
186+
`,
187+
},
188+
} {
189+
t.Run(tc.name, func(t *testing.T) {
190+
repoName := "perforce/" + tc.depot
191+
checkPerforceEnvironment(t)
192+
cleanup := createPerforceExternalService(t, tc.depot, tc.useFusion)
193+
t.Cleanup(cleanup)
194+
195+
err := client.WaitForReposToBeCloned(repoName)
196+
if err != nil {
197+
t.Fatal(err)
198+
}
199+
200+
blob, err := client.GitBlob(repoName, "master", tc.blobPath)
201+
if err != nil {
202+
t.Fatal(err)
203+
}
204+
205+
assert.Equal(t, tc.wantBlob, blob)
206+
})
189207
}
190-
191-
t.Run("git-p4", func(t *testing.T) {
192-
run(t, false)
193-
})
194-
t.Run("p4-fusion", func(t *testing.T) {
195-
run(t, true)
196-
})
197208
}
198209

199210
func checkPerforceEnvironment(t *testing.T) {
@@ -202,7 +213,10 @@ func checkPerforceEnvironment(t *testing.T) {
202213
}
203214
}
204215

205-
func createPerforceExternalService(t *testing.T, useP4Fusion bool) {
216+
// createPerforceExternalService creates an Perforce external service that
217+
// includes the supplied depot. It returns a function to cleanup after the test
218+
// which will delete the depot from disk and remove the external service.
219+
func createPerforceExternalService(t *testing.T, depot string, useP4Fusion bool) func() {
206220
t.Helper()
207221

208222
type Authorization = struct {
@@ -229,7 +243,7 @@ func createPerforceExternalService(t *testing.T, useP4Fusion bool) {
229243
P4Port: *perforcePort,
230244
P4User: *perforceUser,
231245
P4Password: *perforcePassword,
232-
Depots: []string{"//test-perms/"},
246+
Depots: []string{"//" + depot + "/"},
233247
RepositoryPathPattern: "perforce/{depot}",
234248
FusionClient: FusionClient{
235249
Enabled: useP4Fusion,
@@ -246,7 +260,16 @@ func createPerforceExternalService(t *testing.T, useP4Fusion bool) {
246260
if err != nil && !strings.Contains(err.Error(), "/sync-external-service") {
247261
t.Fatal(err)
248262
}
249-
removeExternalServiceAfterTest(t, esID)
263+
264+
return func() {
265+
if err := client.DeleteRepoFromDiskByName("perforce/" + depot); err != nil {
266+
t.Fatalf("removing depot from disk: %v", err)
267+
}
268+
269+
if err := client.DeleteExternalService(esID, false); err != nil {
270+
t.Fatalf("removing external service: %v", err)
271+
}
272+
}
250273
}
251274

252275
func TestExternalService_AsyncDeletion(t *testing.T) {

dev/gqltest/sub_repo_permissions_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,16 @@ import (
1515

1616
const (
1717
perforceRepoName = "perforce/test-perms"
18+
testPermsDepot = "test-perms"
1819
aliceEmail = "[email protected]"
1920
aliceUsername = "alice"
2021
)
2122

2223
func TestSubRepoPermissionsPerforce(t *testing.T) {
2324
checkPerforceEnvironment(t)
2425
enableSubRepoPermissions(t)
25-
createPerforceExternalService(t, false)
26+
cleanup := createPerforceExternalService(t, testPermsDepot, false)
27+
t.Cleanup(cleanup)
2628
userClient, repoName := createTestUserAndWaitForRepo(t)
2729

2830
// Test cases
@@ -81,7 +83,8 @@ func TestSubRepoPermissionsPerforce(t *testing.T) {
8183
func TestSubRepoPermissionsSearch(t *testing.T) {
8284
checkPerforceEnvironment(t)
8385
enableSubRepoPermissions(t)
84-
createPerforceExternalService(t, false)
86+
cleanup := createPerforceExternalService(t, testPermsDepot, false)
87+
t.Cleanup(cleanup)
8588
userClient, _ := createTestUserAndWaitForRepo(t)
8689

8790
err := client.WaitForReposToBeIndexed(perforceRepoName)

0 commit comments

Comments
 (0)