Skip to content

Commit 951d684

Browse files
JoeWang1127JoeWang1127
authored
chore(internal/librarian): refactor command runner (#1746)
Extract common logic when initiating `generateRunner` and `initRunner`. Extract common test functions. Updates #1008
1 parent c9ba07e commit 951d684

File tree

6 files changed

+182
-145
lines changed

6 files changed

+182
-145
lines changed

‎internal/librarian/command.go‎

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,76 @@ import (
2424
"strings"
2525
"time"
2626

27+
"github.com/googleapis/librarian/internal/docker"
28+
2729
"github.com/googleapis/librarian/internal/config"
2830
"github.com/googleapis/librarian/internal/github"
2931
"github.com/googleapis/librarian/internal/gitrepo"
3032
)
3133

34+
type commandRunner struct {
35+
cfg *config.Config
36+
repo gitrepo.Repository
37+
sourceRepo gitrepo.Repository
38+
state *config.LibrarianState
39+
ghClient GitHubClient
40+
containerClient ContainerClient
41+
workRoot string
42+
image string
43+
}
44+
45+
func newCommandRunner(cfg *config.Config) (*commandRunner, error) {
46+
if cfg.APISource == "" {
47+
cfg.APISource = "https://github.com/googleapis/googleapis"
48+
}
49+
sourceRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.APISource, cfg.CI)
50+
if err != nil {
51+
return nil, err
52+
}
53+
54+
languageRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.Repo, cfg.CI)
55+
if err != nil {
56+
return nil, err
57+
}
58+
state, err := loadRepoState(languageRepo, sourceRepo.GetDir())
59+
if err != nil {
60+
return nil, err
61+
}
62+
image := deriveImage(cfg.Image, state)
63+
64+
var gitRepo *github.Repository
65+
if isURL(cfg.Repo) {
66+
gitRepo, err = github.ParseURL(cfg.Repo)
67+
if err != nil {
68+
return nil, fmt.Errorf("failed to parse repo url: %w", err)
69+
}
70+
} else {
71+
gitRepo, err = github.FetchGitHubRepoFromRemote(languageRepo)
72+
if err != nil {
73+
return nil, fmt.Errorf("failed to get GitHub repo from remote: %w", err)
74+
}
75+
}
76+
ghClient, err := github.NewClient(cfg.GitHubToken, gitRepo)
77+
if err != nil {
78+
return nil, fmt.Errorf("failed to create GitHub client: %w", err)
79+
}
80+
81+
container, err := docker.New(cfg.WorkRoot, image, cfg.UserUID, cfg.UserGID)
82+
if err != nil {
83+
return nil, err
84+
}
85+
return &commandRunner{
86+
cfg: cfg,
87+
workRoot: cfg.WorkRoot,
88+
repo: languageRepo,
89+
sourceRepo: sourceRepo,
90+
state: state,
91+
image: image,
92+
ghClient: ghClient,
93+
containerClient: container,
94+
}, nil
95+
}
96+
3297
func cloneOrOpenRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error) {
3398
if repo == "" {
3499
return nil, errors.New("repo must be specified")
@@ -59,11 +124,11 @@ func cloneOrOpenRepo(workRoot, repo, ci string) (*gitrepo.LocalRepository, error
59124
if err != nil {
60125
return nil, err
61126
}
62-
clean, err := githubRepo.IsClean()
127+
cleanRepo, err := githubRepo.IsClean()
63128
if err != nil {
64129
return nil, err
65130
}
66-
if !clean {
131+
if !cleanRepo {
67132
return nil, fmt.Errorf("%s repo must be clean", repo)
68133
}
69134
return githubRepo, nil

‎internal/librarian/generate.go‎

Lines changed: 9 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"github.com/googleapis/librarian/internal/cli"
3030
"github.com/googleapis/librarian/internal/config"
3131
"github.com/googleapis/librarian/internal/docker"
32-
"github.com/googleapis/librarian/internal/github"
3332
"github.com/googleapis/librarian/internal/gitrepo"
3433
)
3534

@@ -99,54 +98,19 @@ type generateRunner struct {
9998
}
10099

101100
func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
102-
if cfg.APISource == "" {
103-
cfg.APISource = "https://github.com/googleapis/googleapis"
104-
}
105-
sourceRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.APISource, cfg.CI)
106-
if err != nil {
107-
return nil, err
108-
}
109-
110-
languageRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.Repo, cfg.CI)
111-
if err != nil {
112-
return nil, err
113-
}
114-
state, err := loadRepoState(languageRepo, sourceRepo.GetDir())
115-
if err != nil {
116-
return nil, err
117-
}
118-
image := deriveImage(cfg.Image, state)
119-
120-
var gitRepo *github.Repository
121-
if isURL(cfg.Repo) {
122-
gitRepo, err = github.ParseURL(cfg.Repo)
123-
if err != nil {
124-
return nil, fmt.Errorf("failed to parse repo url: %w", err)
125-
}
126-
} else {
127-
gitRepo, err = github.FetchGitHubRepoFromRemote(languageRepo)
128-
if err != nil {
129-
return nil, fmt.Errorf("failed to get GitHub repo from remote: %w", err)
130-
}
131-
}
132-
ghClient, err := github.NewClient(cfg.GitHubToken, gitRepo)
133-
if err != nil {
134-
return nil, fmt.Errorf("failed to create GitHub client: %w", err)
135-
}
136-
137-
container, err := docker.New(cfg.WorkRoot, image, cfg.UserUID, cfg.UserGID)
101+
runner, err := newCommandRunner(cfg)
138102
if err != nil {
139103
return nil, err
140104
}
141105
return &generateRunner{
142-
cfg: cfg,
143-
workRoot: cfg.WorkRoot,
144-
repo: languageRepo,
145-
sourceRepo: sourceRepo,
146-
state: state,
147-
image: image,
148-
ghClient: ghClient,
149-
containerClient: container,
106+
cfg: runner.cfg,
107+
workRoot: runner.workRoot,
108+
repo: runner.repo,
109+
sourceRepo: runner.sourceRepo,
110+
state: runner.state,
111+
image: runner.image,
112+
ghClient: runner.ghClient,
113+
containerClient: runner.containerClient,
150114
}, nil
151115
}
152116

‎internal/librarian/generate_test.go‎

Lines changed: 0 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"context"
1919
"errors"
2020
"os"
21-
"os/exec"
2221
"path/filepath"
2322
"regexp"
2423
"sort"
@@ -28,7 +27,6 @@ import (
2827
"github.com/google/go-cmp/cmp"
2928
"github.com/googleapis/librarian/internal/config"
3029
"github.com/googleapis/librarian/internal/gitrepo"
31-
"gopkg.in/yaml.v3"
3230
)
3331

3432
func TestRunGenerateCommand(t *testing.T) {
@@ -453,46 +451,6 @@ func TestNewGenerateRunner(t *testing.T) {
453451
} {
454452
t.Run(test.name, func(t *testing.T) {
455453
t.Parallel()
456-
// We need to create a fake state and config file for the test to pass.
457-
if test.cfg.Repo != "" && !isURL(test.cfg.Repo) {
458-
stateFile := filepath.Join(test.cfg.Repo, config.LibrarianDir, pipelineStateFile)
459-
460-
if err := os.MkdirAll(filepath.Dir(stateFile), 0755); err != nil {
461-
t.Fatalf("os.MkdirAll() = %v", err)
462-
}
463-
464-
state := &config.LibrarianState{
465-
Image: "some/image:v1.2.3",
466-
Libraries: []*config.LibraryState{
467-
{
468-
ID: "some-library",
469-
APIs: []*config.API{
470-
{
471-
Path: "some/api",
472-
ServiceConfig: "api_config.yaml",
473-
Status: config.StatusExisting,
474-
},
475-
},
476-
SourceRoots: []string{"src/a"},
477-
},
478-
},
479-
}
480-
b, err := yaml.Marshal(state)
481-
if err != nil {
482-
t.Fatalf("yaml.Marshal() = %v", err)
483-
}
484-
485-
if err := os.WriteFile(stateFile, b, 0644); err != nil {
486-
t.Fatalf("os.WriteFile(%q, ...) = %v", stateFile, err)
487-
}
488-
configFile := filepath.Join(test.cfg.Repo, config.LibrarianDir, pipelineConfigFile)
489-
if err := os.WriteFile(configFile, []byte("{}"), 0644); err != nil {
490-
t.Fatalf("os.WriteFile(%q, ...) = %v", configFile, err)
491-
}
492-
runGit(t, test.cfg.Repo, "add", ".")
493-
runGit(t, test.cfg.Repo, "commit", "-m", "add config")
494-
}
495-
496454
if test.cfg.APISource == "" && test.cfg.WorkRoot != "" {
497455
if test.name == "clone googleapis fails" {
498456
// The function will try to clone googleapis into the current work directory.
@@ -1647,50 +1605,3 @@ func TestCleanAndCopyLibrary(t *testing.T) {
16471605
})
16481606
}
16491607
}
1650-
1651-
// newTestGitRepo creates a new git repository in a temporary directory.
1652-
func newTestGitRepo(t *testing.T) gitrepo.Repository {
1653-
t.Helper()
1654-
return newTestGitRepoWithState(t, true)
1655-
}
1656-
1657-
// newTestGitRepo creates a new git repository in a temporary directory.
1658-
func newTestGitRepoWithState(t *testing.T, writeState bool) gitrepo.Repository {
1659-
t.Helper()
1660-
dir := t.TempDir()
1661-
remoteURL := "https://github.com/googleapis/librarian.git"
1662-
runGit(t, dir, "init")
1663-
runGit(t, dir, "config", "user.email", "[email protected]")
1664-
runGit(t, dir, "config", "user.name", "Test User")
1665-
if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("test"), 0644); err != nil {
1666-
t.Fatalf("os.WriteFile: %v", err)
1667-
}
1668-
if writeState {
1669-
// Create an empty state.yaml file
1670-
stateDir := filepath.Join(dir, config.LibrarianDir)
1671-
if err := os.MkdirAll(stateDir, 0755); err != nil {
1672-
t.Fatalf("os.MkdirAll: %v", err)
1673-
}
1674-
stateFile := filepath.Join(stateDir, "state.yaml")
1675-
if err := os.WriteFile(stateFile, []byte(""), 0644); err != nil {
1676-
t.Fatalf("os.WriteFile: %v", err)
1677-
}
1678-
}
1679-
runGit(t, dir, "add", ".")
1680-
runGit(t, dir, "commit", "-m", "initial commit")
1681-
runGit(t, dir, "remote", "add", "origin", remoteURL)
1682-
repo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{Dir: dir})
1683-
if err != nil {
1684-
t.Fatalf("gitrepo.Open(%q) = %v", dir, err)
1685-
}
1686-
return repo
1687-
}
1688-
1689-
func runGit(t *testing.T, dir string, args ...string) {
1690-
t.Helper()
1691-
cmd := exec.Command("git", args...)
1692-
cmd.Dir = dir
1693-
if err := cmd.Run(); err != nil {
1694-
t.Fatalf("git %v: %v", args, err)
1695-
}
1696-
}

‎internal/librarian/init.go‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"github.com/googleapis/librarian/internal/cli"
2323
"github.com/googleapis/librarian/internal/config"
24+
"github.com/googleapis/librarian/internal/gitrepo"
2425
)
2526

2627
// cmdInit is the command for the `release init` subcommand.
@@ -44,22 +45,37 @@ func init() {
4445
fs := cmdInit.Flags
4546
cfg := cmdInit.Config
4647

47-
addFlagRepo(fs, cfg)
48+
addFlagPush(fs, cfg)
4849
addFlagImage(fs, cfg)
4950
addFlagLibrary(fs, cfg)
51+
addFlagRepo(fs, cfg)
5052
}
5153

5254
type initRunner struct {
53-
cfg *config.Config
55+
cfg *config.Config
56+
repo gitrepo.Repository
57+
sourceRepo gitrepo.Repository
58+
state *config.LibrarianState
59+
ghClient GitHubClient
60+
containerClient ContainerClient
61+
workRoot string
62+
image string
5463
}
5564

5665
func newInitRunner(cfg *config.Config) (*initRunner, error) {
57-
if ok, err := cfg.IsValid(); !ok || err != nil {
58-
return nil, fmt.Errorf("invalid config: %w", err)
66+
runner, err := newCommandRunner(cfg)
67+
if err != nil {
68+
return nil, fmt.Errorf("failed to create init runner: %w", err)
5969
}
60-
6170
return &initRunner{
62-
cfg: cfg,
71+
cfg: runner.cfg,
72+
workRoot: runner.workRoot,
73+
repo: runner.repo,
74+
sourceRepo: runner.sourceRepo,
75+
state: runner.state,
76+
image: runner.image,
77+
ghClient: runner.ghClient,
78+
containerClient: runner.containerClient,
6379
}, nil
6480
}
6581

‎internal/librarian/init_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
)
2424

2525
func TestNewInitRunner(t *testing.T) {
26+
t.Parallel()
2627
testcases := []struct {
2728
name string
2829
cfg *config.Config
@@ -32,16 +33,20 @@ func TestNewInitRunner(t *testing.T) {
3233
{
3334
name: "valid config",
3435
cfg: &config.Config{
35-
Repo: "/tmp/repo",
36+
API: "some/api",
37+
APISource: newTestGitRepo(t).GetDir(),
38+
Repo: newTestGitRepo(t).GetDir(),
39+
WorkRoot: t.TempDir(),
40+
Image: "gcr.io/test/test-image",
3641
},
3742
},
3843
{
3944
name: "invalid config",
4045
cfg: &config.Config{
41-
Push: true,
46+
APISource: newTestGitRepo(t).GetDir(),
4247
},
4348
wantErr: true,
44-
wantErrMsg: "invalid config",
49+
wantErrMsg: "failed to create init runner",
4550
},
4651
}
4752
for _, test := range testcases {

0 commit comments

Comments
 (0)