Skip to content

Commit 955052c

Browse files
catchyzhengcatchyzheng
authored
feat: clone googleapis if apisource is unspecified (#1589)
Set api-source value to https://github.com/googleapis/googleapis when the flag is unspecified. Also clone googleapis repo. fix: #1512
1 parent c575ba3 commit 955052c

File tree

5 files changed

+86
-14
lines changed

5 files changed

+86
-14
lines changed

‎generate_e2e_test.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ func TestRunGenerate(t *testing.T) {
3434
const (
3535
repo = "repo"
3636
initialRepoStateDir = "testdata/e2e/generate/repo_init"
37+
APISourceRepo = "apisource"
3738
localAPISource = "testdata/e2e/generate/api_root"
3839
)
3940
t.Parallel()
@@ -55,8 +56,12 @@ func TestRunGenerate(t *testing.T) {
5556
t.Run(test.name, func(t *testing.T) {
5657
workRoot := filepath.Join(t.TempDir())
5758
repo := filepath.Join(workRoot, repo)
59+
APISourceRepo := filepath.Join(workRoot, APISourceRepo)
5860
if err := prepareTest(t, repo, workRoot, initialRepoStateDir); err != nil {
59-
t.Fatalf("prepare test error = %v", err)
61+
t.Fatalf("languageRepo prepare test error = %v", err)
62+
}
63+
if err := prepareTest(t, APISourceRepo, workRoot, localAPISource); err != nil {
64+
t.Fatalf("APISouceRepo prepare test error = %v", err)
6065
}
6166

6267
cmd := exec.Command(
@@ -67,7 +72,7 @@ func TestRunGenerate(t *testing.T) {
6772
fmt.Sprintf("--api=%s", test.api),
6873
fmt.Sprintf("--output=%s", workRoot),
6974
fmt.Sprintf("--repo=%s", repo),
70-
fmt.Sprintf("--api-source=%s", localAPISource),
75+
fmt.Sprintf("--api-source=%s", APISourceRepo),
7176
)
7277
cmd.Stderr = os.Stderr
7378
cmd.Stdout = os.Stdout
@@ -109,6 +114,7 @@ func TestRunConfigure(t *testing.T) {
109114
localRepoDir = "testdata/e2e/configure/repo"
110115
initialRepoStateDir = "testdata/e2e/configure/repo_init"
111116
repo = "repo"
117+
APISourceRepo = "apisource"
112118
)
113119
for _, test := range []struct {
114120
name string
@@ -138,9 +144,13 @@ func TestRunConfigure(t *testing.T) {
138144
t.Parallel()
139145
workRoot := filepath.Join(os.TempDir(), fmt.Sprintf("rand-%d", rand.Intn(1000)))
140146
repo := filepath.Join(workRoot, repo)
147+
APISourceRepo := filepath.Join(workRoot, APISourceRepo)
141148
if err := prepareTest(t, repo, workRoot, initialRepoStateDir); err != nil {
142149
t.Fatalf("prepare test error = %v", err)
143150
}
151+
if err := prepareTest(t, APISourceRepo, workRoot, test.apiSource); err != nil {
152+
t.Fatalf("APISouceRepo prepare test error = %v", err)
153+
}
144154

145155
cmd := exec.Command(
146156
"go",
@@ -150,7 +160,7 @@ func TestRunConfigure(t *testing.T) {
150160
fmt.Sprintf("--api=%s", test.api),
151161
fmt.Sprintf("--output=%s", workRoot),
152162
fmt.Sprintf("--repo=%s", repo),
153-
fmt.Sprintf("--api-source=%s", test.apiSource),
163+
fmt.Sprintf("--api-source=%s", APISourceRepo),
154164
fmt.Sprintf("--library=%s", test.library),
155165
)
156166
cmd.Stderr = os.Stderr

‎internal/librarian/command.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func deriveRepoPath(repoFlag string) (string, error) {
4646
return wd, nil
4747
}
4848

49-
func cloneOrOpenLanguageRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error) {
49+
func cloneOrOpenRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error) {
5050
if repo == "" {
5151
return nil, errors.New("repo must be specified")
5252
}
@@ -69,21 +69,21 @@ func cloneOrOpenLanguageRepo(workRoot, repo, ci string) (*gitrepo.LocalRepositor
6969
if err != nil {
7070
return nil, err
7171
}
72-
languageRepo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{
72+
githubRepo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{
7373
Dir: absRepoRoot,
7474
CI: ci,
7575
})
7676
if err != nil {
7777
return nil, err
7878
}
79-
clean, err := languageRepo.IsClean()
79+
clean, err := githubRepo.IsClean()
8080
if err != nil {
8181
return nil, err
8282
}
8383
if !clean {
84-
return nil, errors.New("language repo must be clean")
84+
return nil, fmt.Errorf("%s repo must be clean", repo)
8585
}
86-
return languageRepo, nil
86+
return githubRepo, nil
8787
}
8888

8989
func deriveImage(imageOverride string, state *config.LibrarianState) string {

‎internal/librarian/command_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ func TestCloneOrOpenLanguageRepo(t *testing.T) {
383383
}
384384
}()
385385

386-
repo, err := cloneOrOpenLanguageRepo(workRoot, test.repo, test.ci)
386+
repo, err := cloneOrOpenRepo(workRoot, test.repo, test.ci)
387387
if test.wantErr {
388388
if err == nil {
389389
t.Error("cloneOrOpenLanguageRepo() expected an error but got nil")

‎internal/librarian/generate.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,20 @@ func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
112112
if err != nil {
113113
return nil, err
114114
}
115-
repo, err := cloneOrOpenLanguageRepo(workRoot, cfg.Repo, cfg.CI)
115+
116+
apiSource := cfg.APISource
117+
if apiSource == "" {
118+
apiSource = "https://github.com/googleapis/googleapis"
119+
}
120+
if _, err := cloneOrOpenRepo(workRoot, apiSource, cfg.CI); err != nil {
121+
return nil, err
122+
}
123+
124+
languageRepo, err := cloneOrOpenRepo(workRoot, cfg.Repo, cfg.CI)
116125
if err != nil {
117126
return nil, err
118127
}
119-
state, err := loadRepoState(repo, cfg.APISource)
128+
state, err := loadRepoState(languageRepo, cfg.APISource)
120129
if err != nil {
121130
return nil, err
122131
}
@@ -141,7 +150,7 @@ func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
141150
return &generateRunner{
142151
cfg: cfg,
143152
workRoot: workRoot,
144-
repo: repo,
153+
repo: languageRepo,
145154
state: state,
146155
image: image,
147156
ghClient: ghClient,

‎internal/librarian/generate_test.go‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,22 @@ func TestNewGenerateRunner(t *testing.T) {
324324
name: "valid config",
325325
cfg: &config.Config{
326326
API: "some/api",
327-
APISource: t.TempDir(),
327+
APISource: newTestGitRepo(t).GetDir(),
328+
Repo: newTestGitRepo(t).GetDir(),
329+
WorkRoot: t.TempDir(),
330+
Image: "gcr.io/test/test-image",
331+
},
332+
},
333+
{
334+
name: "invalid api source",
335+
cfg: &config.Config{
336+
API: "some/api",
337+
APISource: t.TempDir(), // Not a git repo
328338
Repo: newTestGitRepo(t).GetDir(),
329339
WorkRoot: t.TempDir(),
330340
Image: "gcr.io/test/test-image",
331341
},
342+
wantErr: true,
332343
},
333344
{
334345
name: "missing image",
@@ -345,13 +356,24 @@ func TestNewGenerateRunner(t *testing.T) {
345356
name: "valid config with github token",
346357
cfg: &config.Config{
347358
API: "some/api",
348-
APISource: t.TempDir(),
359+
APISource: newTestGitRepo(t).GetDir(),
349360
Repo: newTestGitRepo(t).GetDir(),
350361
WorkRoot: t.TempDir(),
351362
Image: "gcr.io/test/test-image",
352363
GitHubToken: "gh-token",
353364
},
354365
},
366+
{
367+
name: "clone googleapis fails",
368+
cfg: &config.Config{
369+
API: "some/api",
370+
APISource: "", // This will trigger the clone of googleapis
371+
Repo: newTestGitRepo(t).GetDir(),
372+
WorkRoot: t.TempDir(),
373+
Image: "gcr.io/test/test-image",
374+
},
375+
wantErr: true,
376+
},
355377
} {
356378
t.Run(test.name, func(t *testing.T) {
357379
t.Parallel()
@@ -362,6 +384,7 @@ func TestNewGenerateRunner(t *testing.T) {
362384
if err := os.MkdirAll(filepath.Dir(stateFile), 0755); err != nil {
363385
t.Fatalf("os.MkdirAll() = %v", err)
364386
}
387+
365388
state := &config.LibrarianState{
366389
Image: "some/image:v1.2.3",
367390
Libraries: []*config.LibraryState{
@@ -376,6 +399,7 @@ func TestNewGenerateRunner(t *testing.T) {
376399
if err != nil {
377400
t.Fatalf("yaml.Marshal() = %v", err)
378401
}
402+
379403
if err := os.WriteFile(stateFile, b, 0644); err != nil {
380404
t.Fatalf("os.WriteFile(%q, ...) = %v", stateFile, err)
381405
}
@@ -387,6 +411,35 @@ func TestNewGenerateRunner(t *testing.T) {
387411
runGit(t, test.cfg.Repo, "commit", "-m", "add config")
388412
}
389413

414+
if test.cfg.APISource == "" && test.cfg.WorkRoot != "" {
415+
if test.name == "clone googleapis fails" {
416+
// The function will try to clone googleapis into the workroot.
417+
// To make it fail, create a non-empty, non-git directory.
418+
googleapisDir := filepath.Join(test.cfg.WorkRoot, "googleapis")
419+
if err := os.MkdirAll(googleapisDir, 0755); err != nil {
420+
t.Fatalf("os.MkdirAll() = %v", err)
421+
}
422+
if err := os.WriteFile(filepath.Join(googleapisDir, "some-file"), []byte("foo"), 0644); err != nil {
423+
t.Fatalf("os.WriteFile() = %v", err)
424+
}
425+
} else {
426+
// The function will try to clone googleapis into the workroot.
427+
// To prevent a real clone, we can pre-create a fake googleapis repo.
428+
googleapisDir := filepath.Join(test.cfg.WorkRoot, "googleapis")
429+
if err := os.MkdirAll(googleapisDir, 0755); err != nil {
430+
t.Fatalf("os.MkdirAll() = %v", err)
431+
}
432+
runGit(t, googleapisDir, "init")
433+
runGit(t, googleapisDir, "config", "user.email", "[email protected]")
434+
runGit(t, googleapisDir, "config", "user.name", "Test User")
435+
if err := os.WriteFile(filepath.Join(googleapisDir, "README.md"), []byte("test"), 0644); err != nil {
436+
t.Fatalf("os.WriteFile: %v", err)
437+
}
438+
runGit(t, googleapisDir, "add", "README.md")
439+
runGit(t, googleapisDir, "commit", "-m", "initial commit")
440+
}
441+
}
442+
390443
_, err := newGenerateRunner(test.cfg)
391444
if (err != nil) != test.wantErr {
392445
t.Errorf("newGenerateRunner() error = %v, wantErr %v", err, test.wantErr)

0 commit comments

Comments
 (0)