From ea72b4ca0e55aa2d9470682ebdd2c72270d409d6 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Sun, 30 Mar 2025 10:50:46 -0400 Subject: [PATCH] Add src/... to test_rules_scala, fix Windows break Adds `src/...` to the `bazel build` and `bazel test` commands in `test_rules_scala.sh` to follow up on #1719 and #1721. Also includes: - Adding `--test_output=errors` to each `bazel test` invocation to make failure messages visible in the CI logs. - Joining the `contents` lines in `WorkerTest.testPersistentWorkerArgsfile` using the `line.separator` system property to fix a test failure on Windows. - Swapping the arguments of `assertEquals()` assertions to `expected, actual` instead of `actual, expected` to fit the assertion failure messages. --- After adding `src/...` to the `bazel test` commands in `test_rules_scala.sh` the first time, the Windows build failed with: - https://buildkite.com/bazel/rules-scala-scala/builds/5511#0195e775-498c-4ae5-b308-34cc70da65c4/75-257 ```txt //src/java/io/bazel/rulesscala/worker:worker_test FAILED in 1.1s C:/.../testlogs/src/java/io/bazel/rulesscala/worker/worker_test/test.log Executed 120 out of 120 tests: 119 tests pass and 1 fails locally. Test "bazel test src/... test/..." failed (18 sec) Traceback (most recent call last): File "c:\b\bk-windows-gd7g\bazel\rules-scala-scala\bazelci.py", line 4528, in sys.exit(main()) ^^^^^^ File "c:\b\bk-windows-gd7g\bazel\rules-scala-scala\bazelci.py", line 4496, in main execute_commands( File "c:\b\bk-windows-gd7g\bazel\rules-scala-scala\bazelci.py", line 1232, in execute_commands PrepareRepoInCwd(True, initial_setup=True) File "c:\b\bk-windows-gd7g\bazel\rules-scala-scala\bazelci.py", line 1228, in PrepareRepoInCwd execute_batch_commands(task_config.get("batch_commands", None), print_cmd_groups) File "c:\b\bk-windows-gd7g\bazel\rules-scala-scala\bazelci.py", line 1741, in execute_batch_commands return subprocess.run(batch_commands, shell=True, check=True, env=os.environ).returncode ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\python3\Lib\subprocess.py", line 571, in run raise CalledProcessError(retcode, process.args, subprocess.CalledProcessError: Command 'set PATH=/usr/bin;%PATH%&bash -lc "pacman --noconfirm --needed -S libxml2"&bash test_rules_scala.sh' returned non-zero exit status 3 ``` Updating the `bazel test` command to add `--test_output=errors` made the actual failure visible: - https://buildkite.com/bazel/rules-scala-scala/builds/5513#0195e78b-6abc-42ea-95ee-8ddce41a64fb ```txt FAIL: //src/java/io/bazel/rulesscala/worker:worker_test (see C:/.../testlogs/src/java/io/bazel/rulesscala/worker/worker_test/test.log) INFO: From Testing //src/java/io/bazel/rulesscala/worker:worker_test: ===== Test output for //src/java/io/bazel/rulesscala/worker:worker_test: JUnit4 Test Runner ...E Time: 0.432 There was 1 failure: 1) testPersistentWorkerArgsfile(io.bazel.rulesscala.worker.WorkerTest) org.junit.ComparisonFailure: expected: but was: at org.junit.Assert.assertEquals(Assert.java:117) at org.junit.Assert.assertEquals(Assert.java:146) at io.bazel.rulesscala.worker.WorkerTest.testPersistentWorkerArgsfile(WorkerTest.java:73) FAILURES!!! Tests run: 3, Failures: 1 ``` This was due to the `line.separator` system property being `\r\n` on Windows, and `\n` on every other platform. Notice the `]ome arg` line, and the fact that this appeared as the `expected:` value in the assertion failure message. --- .../io/bazel/rulesscala/worker/WorkerTest.java | 11 ++++++++--- test_rules_scala.sh | 15 ++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/java/io/bazel/rulesscala/worker/WorkerTest.java b/src/java/io/bazel/rulesscala/worker/WorkerTest.java index 4419ee2ea..80b60ddbf 100644 --- a/src/java/io/bazel/rulesscala/worker/WorkerTest.java +++ b/src/java/io/bazel/rulesscala/worker/WorkerTest.java @@ -55,7 +55,12 @@ public void work(String[] args) { } }; - String contents = "line 1\n--flag_1\nsome arg\n"; + String contents = String.join( + System.getProperty("line.separator"), + "line 1", + "--flag_1", + "some arg", + ""); // The output will always have a line separator at the end. Files.write(tmpFile, contents.getBytes(StandardCharsets.UTF_8)); @@ -69,8 +74,8 @@ public void work(String[] args) { WorkerProtocol.WorkResponse response = WorkerProtocol.WorkResponse.parseDelimitedFrom(helper.responseIn); - assertEquals(response.getExitCode(), 0); - assertEquals(response.getOutput(), contents); + assertEquals(0, response.getExitCode()); + assertEquals(contents, response.getOutput()); } finally { Files.deleteIfExists(tmpFile); } diff --git a/test_rules_scala.sh b/test_rules_scala.sh index d05abddaa..e1c362c06 100755 --- a/test_rules_scala.sh +++ b/test_rules_scala.sh @@ -11,20 +11,21 @@ test_dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )/test/shell # shellcheck source=./test_runner.sh . "${test_dir}"/test_runner.sh runner=$(get_test_runner "${1:-local}") +test_output_flag="--test_output=errors" . "${test_dir}"/test_bzlmod_macros.sh -$runner bazel build test/... -#$runner bazel build "test/... --all_incompatible_changes" -$runner bazel test test/... -$runner bazel test third_party/... +$runner bazel build "src/... test/..." +#$runner bazel build "src/... test/... --all_incompatible_changes" +$runner bazel test "${test_output_flag} src/... test/..." +$runner bazel test "${test_output_flag} third_party/..." $runner bazel build "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/..." $runner bazel build "--extra_toolchains=//scala:minimal_direct_source_deps -- test/..." #$runner bazel build "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error --all_incompatible_changes -- test/..." -$runner bazel test "--extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/..." -$runner bazel test "--extra_toolchains=//scala:minimal_direct_source_deps -- test/..." +$runner bazel test "${test_output_flag} --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_error -- test/..." +$runner bazel test "${test_output_flag} --extra_toolchains=//scala:minimal_direct_source_deps -- test/..." $runner bazel build "test_expect_failure/missing_direct_deps/internal_deps/... --strict_java_deps=warn --extra_toolchains=//test/toolchains:high_level_transitive_deps_strict_deps_warn" $runner bazel build //test_expect_failure/proto_source_root/... --strict_proto_deps=off -$runner bazel test //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" +$runner bazel test "$test_output_flag" //test/... --extra_toolchains="//test_expect_failure/plus_one_deps:plus_one_deps" $runner bazel build test:ScalaBinaryInGenrule --nolegacy_external_runfiles $runner bazel build //test_statsfile:Simple_statsfile $runner bazel build //test_statsfile:SimpleNoStatsFile_statsfile --extra_toolchains="//test/toolchains:enable_stats_file_disabled_toolchain"