From ab88835b3de9146be9788ba8af8e334223bfc7d5 Mon Sep 17 00:00:00 2001 From: anya Date: Tue, 11 Nov 2025 15:48:50 +0000 Subject: [PATCH 1/3] Enhance Go testing support and add integration tests - Update go_test to ensure proper handling of testing definitions. - Modify please_repo_e2e_test to capture command output for assertions. - Add integration test for testing_flag_test with expected output validation. - Introduce necessary BUILD_FILE configurations for plugins and dependencies. --- build_defs/go.build_defs | 23 +++++++++++++++++-- test/build_defs/e2e.build_defs | 9 ++++++-- test/testing_definition/BUILD | 10 ++++++++ test/testing_definition/test_repo/.plzconfig | 8 +++++++ .../test_repo/plugins/BUILD_FILE | 4 ++++ .../test_repo/sample/BUILD_FILE | 4 ++++ .../test_repo/sample/testing_flag_test.go | 13 +++++++++++ .../test_repo/third_party/go/BUILD_FILE | 3 +++ 8 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 test/testing_definition/test_repo/.plzconfig create mode 100644 test/testing_definition/test_repo/plugins/BUILD_FILE create mode 100644 test/testing_definition/test_repo/sample/BUILD_FILE create mode 100644 test/testing_definition/test_repo/sample/testing_flag_test.go create mode 100644 test/testing_definition/test_repo/third_party/go/BUILD_FILE diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index 8de215fd..6ac01340 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -847,10 +847,29 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps: import_path = "main", _generate_pkg_info = False, ) - cmds, tools = _go_binary_cmds(name, static=static, definitions=definitions, gcov=cgo, test=True) + # Match go test: testing/testing.go:692 (Go 1.25.3) relies on testing.testing=true when linking tests. + testing_definition = "testing.testing=true" + if definitions is None: + test_definitions = [testing_definition] + elif isinstance(definitions, dict): + if "testing.testing" in definitions: + test_definitions = definitions + else: + test_definitions = definitions | {"testing.testing": "true"} + else: + if isinstance(definitions, str): + test_definitions = [definitions] + elif isinstance(definitions, list): + test_definitions = list(definitions) + else: + test_definitions = list(definitions) + if testing_definition not in test_definitions: + test_definitions.append(testing_definition) + + cmds, tools = _go_binary_cmds(name, static=static, definitions=test_definitions, gcov=cgo, test=True) - test_cmd = f'$TEST {flags} 2>&1 | tee $TMP_DIR/test.results' + test_cmd = f'$TEST {flags} 2>&1 | tee -- "$TMP_DIR/test.results"' worker_cmd = f'$(worker {worker})' if worker else "" if worker_cmd: test_cmd = f'{worker_cmd} && {test_cmd} ' diff --git a/test/build_defs/e2e.build_defs b/test/build_defs/e2e.build_defs index a68ed0f9..f74d6a32 100644 --- a/test/build_defs/e2e.build_defs +++ b/test/build_defs/e2e.build_defs @@ -2,11 +2,16 @@ subinclude("///e2e//build_defs:e2e") _please_repo_e2e_test = please_repo_e2e_test -def please_repo_e2e_test(name, expected_output=None, plz_command, repo, labels=[]): +def please_repo_e2e_test(name, expected_output=None, plz_command, repo, labels=[], expect_output_contains=None): + plz_cmd = plz_command.replace("plz ", "plz -o please.PluginRepo:file://$TMP_DIR/$DATA_PLUGIN_REPO -o plugin.go.gotool:$TOOLS_GO -o plugin.go.pleasegotool:$TOOLS_PLEASE_GO ") + # Persist command output so subsequent assertions can inspect it while keeping the original exit code. + wrapped_plz_command = f"( {plz_cmd} >output 2>&1 ); STATUS=$?; cat output; exit $STATUS" + return _please_repo_e2e_test( name = name, expected_output = expected_output, - plz_command = plz_command.replace("plz ", "plz -o please.PluginRepo:file://$TMP_DIR/$DATA_PLUGIN_REPO -o plugin.go.gotool:$TOOLS_GO -o plugin.go.pleasegotool:$TOOLS_PLEASE_GO "), + expect_output_contains = expect_output_contains, + plz_command = wrapped_plz_command, repo = repo, data = { "PLUGIN_REPO": ["//test:export"], diff --git a/test/testing_definition/BUILD b/test/testing_definition/BUILD index 4bdd6436..1dee7442 100644 --- a/test/testing_definition/BUILD +++ b/test/testing_definition/BUILD @@ -1,4 +1,5 @@ subinclude("//build_defs:go") +subinclude("//test/build_defs:e2e") go_test( name = "testing_definition_test", @@ -7,3 +8,12 @@ go_test( "///third_party/go/github.com_stretchr_testify//assert", ], ) + +please_repo_e2e_test( + name = "testing_definition_integration_test", + plz_command = "plz test --show_all_output //sample:testing_flag_test -- -test.v", + repo = "test_repo", + expect_output_contains = { + "output": "testing.Testing() = true", + }, +) diff --git a/test/testing_definition/test_repo/.plzconfig b/test/testing_definition/test_repo/.plzconfig new file mode 100644 index 00000000..438409f7 --- /dev/null +++ b/test/testing_definition/test_repo/.plzconfig @@ -0,0 +1,8 @@ +[Parse] +BuildFileName = BUILD_FILE +BuildFileName = BUILD +PreloadSubincludes = ///go//build_defs:go + +[Plugin "go"] +Target = //plugins:go +Stdlib = //third_party/go:std diff --git a/test/testing_definition/test_repo/plugins/BUILD_FILE b/test/testing_definition/test_repo/plugins/BUILD_FILE new file mode 100644 index 00000000..763befc7 --- /dev/null +++ b/test/testing_definition/test_repo/plugins/BUILD_FILE @@ -0,0 +1,4 @@ +plugin_repo( + name = "go", + revision = "master", +) diff --git a/test/testing_definition/test_repo/sample/BUILD_FILE b/test/testing_definition/test_repo/sample/BUILD_FILE new file mode 100644 index 00000000..ea5b4980 --- /dev/null +++ b/test/testing_definition/test_repo/sample/BUILD_FILE @@ -0,0 +1,4 @@ +go_test( + name = "testing_flag_test", + srcs = ["testing_flag_test.go"], +) diff --git a/test/testing_definition/test_repo/sample/testing_flag_test.go b/test/testing_definition/test_repo/sample/testing_flag_test.go new file mode 100644 index 00000000..45afddfc --- /dev/null +++ b/test/testing_definition/test_repo/sample/testing_flag_test.go @@ -0,0 +1,13 @@ +package sample + +import ( + "fmt" + "testing" +) + +func TestTestingFlag(t *testing.T) { + fmt.Printf("testing.Testing() = %v\n", testing.Testing()) + if !testing.Testing() { + t.Fatalf("expected testing.Testing() to be true") + } +} diff --git a/test/testing_definition/test_repo/third_party/go/BUILD_FILE b/test/testing_definition/test_repo/third_party/go/BUILD_FILE new file mode 100644 index 00000000..f6d6b9a1 --- /dev/null +++ b/test/testing_definition/test_repo/third_party/go/BUILD_FILE @@ -0,0 +1,3 @@ +go_stdlib( + name = "std", +) From cb81c492773ee4c37ad9808b35cefaa25e459306 Mon Sep 17 00:00:00 2001 From: "Xiao, Yujiao" Date: Wed, 18 Feb 2026 15:16:10 +0000 Subject: [PATCH 2/3] Refactor go_test function to simplify definitions handling and update .plzconfig --- build_defs/go.build_defs | 2 -- test/testing_definition/test_repo/.plzconfig | 1 - 2 files changed, 3 deletions(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index 6ac01340..7985bc06 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -859,8 +859,6 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps: else: if isinstance(definitions, str): test_definitions = [definitions] - elif isinstance(definitions, list): - test_definitions = list(definitions) else: test_definitions = list(definitions) if testing_definition not in test_definitions: diff --git a/test/testing_definition/test_repo/.plzconfig b/test/testing_definition/test_repo/.plzconfig index 438409f7..349fc964 100644 --- a/test/testing_definition/test_repo/.plzconfig +++ b/test/testing_definition/test_repo/.plzconfig @@ -1,6 +1,5 @@ [Parse] BuildFileName = BUILD_FILE -BuildFileName = BUILD PreloadSubincludes = ///go//build_defs:go [Plugin "go"] From af08aebf99ec92fe26e88910346dbf88631059ad Mon Sep 17 00:00:00 2001 From: "Xiao, Yujiao" Date: Wed, 18 Feb 2026 15:23:25 +0000 Subject: [PATCH 3/3] Refactor testing definitions in go_test and please_repo_e2e_test for improved output handling --- build_defs/go.build_defs | 26 +++++++------------------- test/build_defs/e2e.build_defs | 14 ++++++++++---- test/testing_definition/BUILD | 4 +--- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index 7985bc06..e79bafed 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -847,24 +847,7 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps: import_path = "main", _generate_pkg_info = False, ) - # Match go test: testing/testing.go:692 (Go 1.25.3) relies on testing.testing=true when linking tests. - testing_definition = "testing.testing=true" - if definitions is None: - test_definitions = [testing_definition] - elif isinstance(definitions, dict): - if "testing.testing" in definitions: - test_definitions = definitions - else: - test_definitions = definitions | {"testing.testing": "true"} - else: - if isinstance(definitions, str): - test_definitions = [definitions] - else: - test_definitions = list(definitions) - if testing_definition not in test_definitions: - test_definitions.append(testing_definition) - - cmds, tools = _go_binary_cmds(name, static=static, definitions=test_definitions, gcov=cgo, test=True) + cmds, tools = _go_binary_cmds(name, static=static, definitions=definitions, gcov=cgo, test=True) test_cmd = f'$TEST {flags} 2>&1 | tee -- "$TMP_DIR/test.results"' @@ -1764,7 +1747,12 @@ def _go_binary_cmds(name, static=False, ldflags='', pkg_config='', definitions=N elif isinstance(definitions, dict): linkerdefs = [k if v is None else f'{k}={v}' for k, v in sorted(definitions.items())] if test: - linkerdefs += ["testing.testBinary=1"] + # testing.testBinary and testing.testing are required by the Go test runtime. + # Match go test: testing/testing.go:692 (Go 1.25.3) relies on testing.testing=true when linking tests. + if "testing.testBinary=1" not in linkerdefs: + linkerdefs += ["testing.testBinary=1"] + if "testing.testing=true" not in linkerdefs: + linkerdefs += ["testing.testing=true"] defs = ' '.join([f'-X "{linkerdef}"' for linkerdef in linkerdefs]) diff --git a/test/build_defs/e2e.build_defs b/test/build_defs/e2e.build_defs index f74d6a32..e43ae9d9 100644 --- a/test/build_defs/e2e.build_defs +++ b/test/build_defs/e2e.build_defs @@ -2,15 +2,21 @@ subinclude("///e2e//build_defs:e2e") _please_repo_e2e_test = please_repo_e2e_test -def please_repo_e2e_test(name, expected_output=None, plz_command, repo, labels=[], expect_output_contains=None): +def please_repo_e2e_test(name, expected_output=None, plz_command, repo, labels=[], expect_stdout_contains=None, expect_stderr_contains=None): plz_cmd = plz_command.replace("plz ", "plz -o please.PluginRepo:file://$TMP_DIR/$DATA_PLUGIN_REPO -o plugin.go.gotool:$TOOLS_GO -o plugin.go.pleasegotool:$TOOLS_PLEASE_GO ") - # Persist command output so subsequent assertions can inspect it while keeping the original exit code. - wrapped_plz_command = f"( {plz_cmd} >output 2>&1 ); STATUS=$?; cat output; exit $STATUS" + # Tee stdout and stderr to separate files while preserving the exit code. + wrapped_plz_command = f"( {plz_cmd} > >(tee stdout.log) 2> >(tee stderr.log >&2) ); STATUS=$?; exit $STATUS" + + expect_output_contains = {} + if expect_stdout_contains: + expect_output_contains["stdout.log"] = expect_stdout_contains + if expect_stderr_contains: + expect_output_contains["stderr.log"] = expect_stderr_contains return _please_repo_e2e_test( name = name, expected_output = expected_output, - expect_output_contains = expect_output_contains, + expect_output_contains = expect_output_contains if expect_output_contains else None, plz_command = wrapped_plz_command, repo = repo, data = { diff --git a/test/testing_definition/BUILD b/test/testing_definition/BUILD index 1dee7442..2ea4549f 100644 --- a/test/testing_definition/BUILD +++ b/test/testing_definition/BUILD @@ -13,7 +13,5 @@ please_repo_e2e_test( name = "testing_definition_integration_test", plz_command = "plz test --show_all_output //sample:testing_flag_test -- -test.v", repo = "test_repo", - expect_output_contains = { - "output": "testing.Testing() = true", - }, + expect_stdout_contains = "testing.Testing() = true", )