Fix proto_compiled_sources with strip_import_prefix (#365)

* Migrate example tests to "test_content" attribute
* Simplify Makefile
* Switch to proto_compile_assets
* be more tricky with unused imports
* Augment test with strip_import_prefix case
* Modify test such that package matches directory structure
* Modify test to not include a package
* Test README

Author: pcj

BUILD.bazel

@@ -64,6 +64,7 @@ filegroup(
         "WORKSPACE",
         "go_deps.bzl",
         "//cmd/gazelle:all_files",
+        "//cmd/gencopy:all_files",
         "//deps:all_files",
         "//language/protobuf:all_files",
         "//pkg:all_files",

Makefile

@@ -1,44 +1,47 @@
-BAZEL := bazel
 
 .PHONY: tidy
 tidy: deps
-	$(BAZEL) run @go_sdk//:bin/go -- mod tidy
-	$(BAZEL) run @go_sdk//:bin/go -- mod vendor
+	bazel run @go_sdk//:bin/go -- mod tidy
+	bazel run @go_sdk//:bin/go -- mod vendor
 	find vendor -name 'BUILD.bazel' | xargs rm
-	$(BAZEL) run //:update_go_deps
-	$(BAZEL) run //:buildifier
-	$(BAZEL) run //:gazelle
+	bazel run //:update_go_deps
+	bazel run //:buildifier
+	bazel run //:gazelle
 
 .PHONY: gazelle
 gazelle:
-	$(BAZEL) run //:gazelle
+	bazel run //:gazelle
 
 .PHONY: deps
 deps:
-	$(BAZEL) build //deps:*
+	bazel build //deps:*
 	cp -f ./bazel-bin/deps/*.bzl deps/
 	chmod 0644 deps/*.bzl
-	$(BAZEL) run //:buildifier -- deps/
+	bazel run //:buildifier -- deps/
 
 .PHONY: site
 site:
-	$(BAZEL) build //example/golden:*
+	bazel build //example/golden:*
 	cp -f ./bazel-bin/example/golden/*.md docs/
 
 .PHONY: golden_test
 golden_test:
-	$(BAZEL) test //example/golden:golden_test
+	bazel test //example/golden:golden_test --test_output=streamed
+
+.PHONY: example_test
+example_test:
+	bazel test //example/golden:proto_compiled_sources_test --test_output=streamed
 
 .PHONY: test
 test:
-	$(BAZEL) test --keep_going //example/... //pkg/... //plugin/... //language/... //rules/... //toolchain/... \
+	bazel test --keep_going //example/... //pkg/... //plugin/... //language/... //rules/... //toolchain/... \
 		--deleted_packages=//plugin/grpc-ecosystem/grpc-gateway
 
 .PHONY: get
 get:
-	$(BAZEL) run @go_sdk//:bin/go -- get github.com/bazelbuild/bazel-gazelle@v0.31.0
-	$(BAZEL) run @go_sdk//:bin/go -- mod download github.com/bazelbuild/buildtools
-	$(BAZEL) run @go_sdk//:bin/go -- mod vendor
+	bazel run @go_sdk//:bin/go -- get github.com/bazelbuild/bazel-gazelle@v0.31.0
+	bazel run @go_sdk//:bin/go -- mod download github.com/bazelbuild/buildtools
+	bazel run @go_sdk//:bin/go -- mod vendor
 
 update_pnpm_lock:
 	# nvm use 18

cmd/examplegen/config.go

@@ -10,7 +10,7 @@ type Config struct {
 	Name        string
 	Label       string
 	TestOut     string
-	TestHeader  string
+	TestContent string
 	MarkdownOut string
 	WorkspaceIn string
 	StripPrefix string

cmd/examplegen/generator.go

@@ -87,7 +87,7 @@ func generateTest(c *Config) error {
 	defer f.Close()
 
 	fmt.Fprintln(f, testHeader)
-	fmt.Fprintln(f, c.TestHeader)
+	fmt.Fprintln(f, c.TestContent)
 
 	fmt.Fprintln(f, "var txtar=`")
 

cmd/examplegen/template.go

@@ -5,19 +5,22 @@ package main
 
 import (
 	"testing"
+	"os"
 
 	"github.com/bazelbuild/rules_go/go/tools/bazel_testing"
+	"github.com/google/go-cmp/cmp"
+)
+
+var (
+	// allow use of os package in other tests
+	_os_Remove = os.Remove
+	// allow use of cmp package in other tests
+	_cmp_Diff = cmp.Diff
 )
 
 func TestMain(m *testing.M) {
 	bazel_testing.TestMain(m, bazel_testing.Args{
 		Main: txtar,
 	})
 }
-
-func TestBuild(t *testing.T) {
-	if err := bazel_testing.RunBazel("build", "..."); err != nil {
-		t.Fatal(err)
-	}
-}
 `

cmd/gencopy/BUILD.bazel

@@ -20,5 +20,21 @@ go_test(
     name = "gencopy_test",
     srcs = ["gencopy_test.go"],
     embed = [":gencopy_lib"],
-    deps = ["@com_github_google_go_cmp//cmp"],
+    deps = [
+        "@bazel_gazelle//testtools:go_default_library",
+        "@com_github_google_go_cmp//cmp",
+    ],
+)
+
+filegroup(
+    name = "all_files",
+    testonly = True,
+    srcs = [
+        "BUILD.bazel",
+        "gencopy.bash.in",
+        "gencopy.bzl",
+        "gencopy.go",
+        "gencopy_test.go",
+    ],
+    visibility = ["//:__pkg__"],
 )

cmd/gencopy/gencopy.go

@@ -1,14 +1,12 @@
 // gencopy is a utility program that copies bazel outputs back into the
 // workspace source tree.  Ideally, you don't have any generated files committed
 // to VCS, but sometimes you do.
-//
 package main
 
 import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
@@ -89,7 +87,7 @@ func copyFile(src, dst string, mode os.FileMode) error {
 	// NOTE: for some reason the io.Copy approach was writing an empty file...
 	// for now OK to copy in-memory
 
-	data, err := ioutil.ReadFile(src)
+	data, err := os.ReadFile(src)
 	if err != nil {
 		return err
 	}
@@ -98,28 +96,18 @@ func copyFile(src, dst string, mode os.FileMode) error {
 		return err
 	}
 
-	return ioutil.WriteFile(dst, data, mode)
+	return os.WriteFile(dst, data, mode)
 }
 
 // readFileAsString reads the given file assumed to be text
 func readFileAsString(filename string) (string, error) {
-	bytes, err := ioutil.ReadFile(filename)
+	bytes, err := os.ReadFile(filename)
 	if err != nil {
 		return "", fmt.Errorf("could not read %s: %v", filename, err)
 	}
 	return string(bytes), nil
 }
 
-func usageHint(cfg *Config, pkg *PackageConfig) string {
-	return fmt.Sprintf(`You may need to regenerate the files (bazel run) using the '.%[2]s' target,
-update the 'srcs = [...]' attribute to include the generated files and re-run the test:
-
-$ bazel run %[1]s.%[2]s
-$ bazel test %[1]s
-
-`, pkg.TargetLabel, cfg.UpdateTargetLabelName)
-}
-
 func check(cfg *Config, pkg *PackageConfig, pairs []*SrcDst) error {
 	for _, pair := range pairs {
 		expected, err := readFileAsString(pair.Src)
@@ -227,7 +215,7 @@ func run(cfg *Config) error {
 }
 
 func readConfig(workspaceRootDirectory string) (*Config, error) {
-	data, err := ioutil.ReadFile(*config)
+	data, err := os.ReadFile(*config)
 	if err != nil {
 		return nil, fmt.Errorf("could not read config file %s: %w", *config, err)
 	}

cmd/gencopy/gencopy_test.go

@@ -1,13 +1,14 @@
 // gencopy is a utility program that copies bazel outputs back into the
 // workspace source tree.  Ideally, you don't have any generated files committed
 // to VCS, but sometimes you do.
-//
 package main
 
 import (
 	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/bazelbuild/bazel-gazelle/testtools"
 	"github.com/google/go-cmp/cmp"
 )
 
@@ -56,6 +57,12 @@ func TestMakePkgSrcDstPair(t *testing.T) {
 			dst:  "file.txt",
 			want: SrcDst{Src: "file.txt", Dst: "/home/file.txt"},
 		},
+		"WorkspaceSubDirectory": {
+			cfg:  Config{WorkspaceRootDirectory: "/home"},
+			src:  "subdir/file.txt",
+			dst:  "subdir/file.txt",
+			want: SrcDst{Src: "subdir/file.txt", Dst: "/home/subdir/file.txt"},
+		},
 		"TargetWorkspaceRoot": {
 			cfg:  Config{WorkspaceRootDirectory: "/home"},
 			pkg:  PackageConfig{TargetWorkspaceRoot: "external/foo"},
@@ -73,3 +80,79 @@ func TestMakePkgSrcDstPair(t *testing.T) {
 		})
 	}
 }
+
+func TestRunPkg(t *testing.T) {
+	for name, tc := range map[string]struct {
+		cfg         Config
+		files, want []testtools.FileSpec
+	}{
+		"degenerate": {},
+		"simple": {
+			// {"extension":"","fileMode":"0644","mode":"update","packageConfigs":[{"generatedFiles":["api/v1/v1_pb2.py"],"sourceFiles":["api/v1/v1_pb2.py"],"targetLabel":"@//api/v1:api_v1_python_compiled_sources","targetPackage":"api/v1","targetWorkspaceRoot":""}],"updateTargetLabelName":"api_v1_python_compiled_sources.update"}
+			cfg: Config{
+				Extension:              "",
+				FileMode:               "0644",
+				Mode:                   "update",
+				WorkspaceRootDirectory: "workspace",
+				UpdateTargetLabelName:  "api_v1_python_compiled_sources.update",
+				PackageConfigs: []*PackageConfig{
+					{
+						GeneratedFiles:      []string{"api/v1/v1_pb2.py"},
+						SourceFiles:         []string{"api/v1/v1_pb2.py"},
+						TargetLabel:         "@//api/v1:api_v1_python_compiled_sources",
+						TargetPackage:       "api/v1",
+						TargetWorkspaceRoot: "gen",
+					},
+				},
+			},
+			files: []testtools.FileSpec{
+				{
+					Path:    "workspace/api/v1/v1_pb2.py",
+					Content: "# generated file api/v1/v1_pb2.py",
+				},
+			},
+			want: []testtools.FileSpec{
+				{
+					Path:    "api/v1/v1_pb2.py",
+					Content: "# generated file api/v1/v1_pb2.py",
+				},
+			},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			dir, cleanup := testtools.CreateFiles(t, tc.files)
+			defer cleanup()
+
+			if err := os.Chdir(dir); err != nil {
+				t.Fatal(err)
+			}
+			listFiles(t, ".")
+			if err := run(&tc.cfg, t.Logf); err != nil {
+				t.Fatal(err)
+			}
+
+			testtools.CheckFiles(t, dir, tc.want)
+		})
+	}
+}
+
+// listFiles - convenience debugging function to log the files under a given dir
+func listFiles(t *testing.T, dir string) error {
+	return filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			t.Logf("%v\n", err)
+			return err
+		}
+		if info.Mode()&os.ModeSymlink > 0 {
+			link, err := os.Readlink(path)
+			if err != nil {
+				return err
+			}
+			t.Logf("%s -> %s", path, link)
+			return nil
+		}
+
+		t.Log(path)
+		return nil
+	})
+}