Skip to content

Commit e450f8f

Browse files
authored
fix(internal/librarian/java): Improve error handling in generate.go and add back test case (#4707)
Improves error handling in generate.go, use sentinel error for checks in tests. Add back temporary removed test case, and removes a redundant test. Fix #4517 Fix #4436
1 parent 6064cf7 commit e450f8f

2 files changed

Lines changed: 30 additions & 9 deletions

File tree

internal/librarian/java/generate.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package java
1717

1818
import (
1919
"context"
20+
"errors"
2021
"fmt"
2122
"os"
2223
"os/exec"
@@ -36,6 +37,13 @@ const (
3637
commonProtos = "google/cloud/common_resources.proto"
3738
)
3839

40+
var (
41+
// errExtractVersion is returned when an API version cannot be extracted from its path.
42+
errExtractVersion = errors.New("failed to extract version")
43+
// errNoProtos is returned when no proto files are found in an API directory.
44+
errNoProtos = errors.New("no protos found")
45+
)
46+
3947
// Generate generates a Java client library.
4048
func Generate(ctx context.Context, cfg *config.Config, library *config.Library, srcs *sources.Sources) error {
4149
outdir, err := filepath.Abs(library.Output)
@@ -49,7 +57,7 @@ func Generate(ctx context.Context, cfg *config.Config, library *config.Library,
4957
return fmt.Errorf("failed to resolve googleapis directory path: %w", err)
5058
}
5159
if err := os.MkdirAll(outdir, 0755); err != nil {
52-
return fmt.Errorf("failed to create output directory: %w", err)
60+
return fmt.Errorf("failed to create output directory %q: %w", outdir, err)
5361
}
5462
for _, api := range library.APIs {
5563
if err := generateAPI(ctx, cfg, api, library, googleapisDir, outdir); err != nil {
@@ -62,7 +70,7 @@ func Generate(ctx context.Context, cfg *config.Config, library *config.Library,
6270
func generateAPI(ctx context.Context, cfg *config.Config, api *config.API, library *config.Library, googleapisDir, outdir string) error {
6371
version := serviceconfig.ExtractVersion(api.Path)
6472
if version == "" {
65-
return fmt.Errorf("failed to generate api: failed to extract version from api path %q", api.Path)
73+
return fmt.Errorf("%s: %w", api.Path, errExtractVersion)
6674
}
6775
javaAPI := resolveJavaAPI(library, api)
6876
p := postProcessParams{
@@ -77,7 +85,7 @@ func generateAPI(ctx context.Context, cfg *config.Config, api *config.API, libra
7785
}
7886
for _, dir := range []string{p.gapicDir, p.grpcDir, p.protoDir} {
7987
if err := os.MkdirAll(dir, 0755); err != nil {
80-
return fmt.Errorf("failed to create directory %s: %w", dir, err)
88+
return fmt.Errorf("failed to create directory %q: %w", dir, err)
8189
}
8290
}
8391

@@ -91,7 +99,7 @@ func generateAPI(ctx context.Context, cfg *config.Config, api *config.API, libra
9199
return fmt.Errorf("failed to find protos: %w", err)
92100
}
93101
if len(apiProtos) == 0 {
94-
return fmt.Errorf("failed to generate api: no protos found in api %q", api.Path)
102+
return fmt.Errorf("%s: %w", api.Path, errNoProtos)
95103
}
96104
p.apiProtos = apiProtos
97105

@@ -246,7 +254,7 @@ func Format(ctx context.Context, library *config.Library) error {
246254

247255
args := append([]string{"--replace"}, files...)
248256
if err := command.Run(ctx, "google-java-format", args...); err != nil {
249-
return fmt.Errorf("formatting failed: %w", err)
257+
return fmt.Errorf("failed to format files: %w", err)
250258
}
251259
return nil
252260
}

internal/librarian/java/generate_test.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ package java
1616

1717
import (
1818
"context"
19+
"errors"
1920
"os"
2021
"path/filepath"
2122
"strings"
23+
"syscall"
2224

2325
"sort"
2426

@@ -409,7 +411,7 @@ func TestGenerateLibrary_Error(t *testing.T) {
409411
name string
410412
library *config.Library
411413
setup func(t *testing.T, library *config.Library)
412-
wantErr string
414+
wantErr error
413415
}{
414416
{
415417
name: "invalid version",
@@ -420,7 +422,18 @@ func TestGenerateLibrary_Error(t *testing.T) {
420422
{Path: "google/cloud/secretmanager"}, // Missing version
421423
},
422424
},
423-
wantErr: "failed to extract version from api path",
425+
wantErr: errExtractVersion,
426+
},
427+
{
428+
name: "no protos found",
429+
library: &config.Library{
430+
Name: "test",
431+
Output: t.TempDir(),
432+
APIs: []*config.API{
433+
{Path: "google/cloud/nonexistent/v1"},
434+
},
435+
},
436+
wantErr: errNoProtos,
424437
},
425438
{
426439
name: "mkdir failure for output dir",
@@ -437,7 +450,7 @@ func TestGenerateLibrary_Error(t *testing.T) {
437450
t.Fatal(err)
438451
}
439452
},
440-
wantErr: "failed to create output directory",
453+
wantErr: syscall.ENOTDIR,
441454
},
442455
} {
443456
t.Run(test.name, func(t *testing.T) {
@@ -446,7 +459,7 @@ func TestGenerateLibrary_Error(t *testing.T) {
446459
}
447460
cfg := &config.Config{Language: "java"}
448461
err := Generate(t.Context(), cfg, test.library, &sources.Sources{Googleapis: googleapisDir})
449-
if err == nil || !strings.Contains(err.Error(), test.wantErr) {
462+
if !errors.Is(err, test.wantErr) {
450463
t.Errorf("generate() error = %v, wantErr %v", err, test.wantErr)
451464
}
452465
})

0 commit comments

Comments
 (0)