Skip to content

Commit 70ce073

Browse files
authored
chore: revert "refactor(librarian): Inject client factories and add test for generate default" (#1919)
The time to run all unit tests increases from 1 mins to 5 mins. Example logs: [Before](https://github.com/googleapis/librarian/actions/runs/17449743710/job/49552035801) [After](https://github.com/googleapis/librarian/actions/runs/17449910373/job/49552495491) Reverts #1918
1 parent a8a38cb commit 70ce073

File tree

6 files changed

+9
-67
lines changed

6 files changed

+9
-67
lines changed

internal/librarian/command.go

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,6 @@ const (
4343
release = "release"
4444
)
4545

46-
// GitHubClientFactory type for creating a GitHubClient.
47-
type GitHubClientFactory func(token string, repo *github.Repository) (GitHubClient, error)
48-
49-
// ContainerClientFactory type for creating a ContainerClient.
50-
type ContainerClientFactory func(workRoot, image, userUID, userGID string) (ContainerClient, error)
51-
5246
type commitInfo struct {
5347
cfg *config.Config
5448
state *config.LibrarianState
@@ -73,20 +67,7 @@ type commandRunner struct {
7367

7468
const defaultAPISourceBranch = "master"
7569

76-
func newCommandRunner(cfg *config.Config, ghClientFactory GitHubClientFactory, containerClientFactory ContainerClientFactory) (*commandRunner, error) {
77-
// If no GitHub client factory is provided, use the default one.
78-
if ghClientFactory == nil {
79-
ghClientFactory = func(token string, repo *github.Repository) (GitHubClient, error) {
80-
return github.NewClient(token, repo)
81-
}
82-
}
83-
// If no container client factory is provided, use the default one.
84-
if containerClientFactory == nil {
85-
containerClientFactory = func(workRoot, image, userUID, userGID string) (ContainerClient, error) {
86-
return docker.New(workRoot, image, userUID, userGID)
87-
}
88-
}
89-
70+
func newCommandRunner(cfg *config.Config) (*commandRunner, error) {
9071
if cfg.APISource == "" {
9172
cfg.APISource = "https://github.com/googleapis/googleapis"
9273
}
@@ -129,12 +110,12 @@ func newCommandRunner(cfg *config.Config, ghClientFactory GitHubClientFactory, c
129110
return nil, fmt.Errorf("failed to get GitHub repo from remote: %w", err)
130111
}
131112
}
132-
ghClient, err := ghClientFactory(cfg.GitHubToken, gitRepo)
113+
ghClient, err := github.NewClient(cfg.GitHubToken, gitRepo)
133114
if err != nil {
134115
return nil, fmt.Errorf("failed to create GitHub client: %w", err)
135116
}
136117

137-
container, err := containerClientFactory(cfg.WorkRoot, image, cfg.UserUID, cfg.UserGID)
118+
container, err := docker.New(cfg.WorkRoot, image, cfg.UserUID, cfg.UserGID)
138119
if err != nil {
139120
return nil, err
140121
}

internal/librarian/generate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ After generation, if the "-push" flag is provided, the changes are committed to
6262
a pull request is created. Otherwise, the changes are left in the local working tree for
6363
inspection.`,
6464
Run: func(ctx context.Context, cfg *config.Config) error {
65-
runner, err := newGenerateRunner(cfg, nil, nil)
65+
runner, err := newGenerateRunner(cfg)
6666
if err != nil {
6767
return err
6868
}
@@ -98,8 +98,8 @@ type generateRunner struct {
9898
image string
9999
}
100100

101-
func newGenerateRunner(cfg *config.Config, ghClientFactory GitHubClientFactory, containerClientFactory ContainerClientFactory) (*generateRunner, error) {
102-
runner, err := newCommandRunner(cfg, ghClientFactory, containerClientFactory)
101+
func newGenerateRunner(cfg *config.Config) (*generateRunner, error) {
102+
runner, err := newCommandRunner(cfg)
103103
if err != nil {
104104
return nil, err
105105
}

internal/librarian/generate_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ func TestNewGenerateRunner(t *testing.T) {
505505
}
506506
}
507507

508-
r, err := newGenerateRunner(test.cfg, nil, nil)
508+
r, err := newGenerateRunner(test.cfg)
509509
if (err != nil) != test.wantErr {
510510
t.Errorf("newGenerateRunner() error = %v, wantErr %v", err, test.wantErr)
511511
}

internal/librarian/librarian_test.go

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030

3131
"github.com/googleapis/librarian/internal/cli"
3232
"github.com/googleapis/librarian/internal/config"
33-
"github.com/googleapis/librarian/internal/github"
3433
"github.com/googleapis/librarian/internal/gitrepo"
3534
"gopkg.in/yaml.v3"
3635

@@ -83,44 +82,6 @@ func TestParentCommands(t *testing.T) {
8382
}
8483
}
8584

86-
func TestGenerate_DefaultBehavior(t *testing.T) {
87-
ctx := context.Background()
88-
89-
// 1. Setup a mock repository with a state file
90-
repo := newTestGitRepoWithState(t, true)
91-
repoDir := repo.GetDir()
92-
93-
t.Chdir(repoDir)
94-
95-
// 2. Override dependency creation to use mocks
96-
mockContainer := &mockContainerClient{
97-
wantLibraryGen: true,
98-
}
99-
mockGH := &mockGitHubClient{}
100-
101-
// 3. Call librarian.Run
102-
cfg := config.New("generate")
103-
cfg.WorkRoot = repoDir
104-
cfg.Repo = repoDir
105-
runner, err := newGenerateRunner(cfg, func(token string, repo *github.Repository) (GitHubClient, error) {
106-
return mockGH, nil
107-
}, func(workRoot, image, userUID, userGID string) (ContainerClient, error) {
108-
return mockContainer, nil
109-
})
110-
if err != nil {
111-
t.Fatalf("newGenerateRunner() failed: %v", err)
112-
}
113-
if err := runner.run(ctx); err != nil {
114-
t.Fatalf("runner.run() failed: %v", err)
115-
}
116-
117-
// 4. Assertions
118-
expectedGenerateCalls := 1
119-
if mockContainer.generateCalls != expectedGenerateCalls {
120-
t.Errorf("Run(ctx, \"generate\"): got %d generate calls, want %d", mockContainer.generateCalls, expectedGenerateCalls)
121-
}
122-
}
123-
12485
func TestIsURL(t *testing.T) {
12586
for _, test := range []struct {
12687
name string

internal/librarian/release_init.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ type initRunner struct {
7878
}
7979

8080
func newInitRunner(cfg *config.Config) (*initRunner, error) {
81-
runner, err := newCommandRunner(cfg, nil, nil)
81+
runner, err := newCommandRunner(cfg)
8282
if err != nil {
8383
return nil, fmt.Errorf("failed to create init runner: %w", err)
8484
}

internal/librarian/tag_and_release.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ type tagAndReleaseRunner struct {
7474
}
7575

7676
func newTagAndReleaseRunner(cfg *config.Config) (*tagAndReleaseRunner, error) {
77-
runner, err := newCommandRunner(cfg, nil, nil)
77+
runner, err := newCommandRunner(cfg)
7878
if err != nil {
7979
return nil, err
8080
}

0 commit comments

Comments
 (0)