From eac6f0613e7725deeb4260643ae1480f31f415a4 Mon Sep 17 00:00:00 2001 From: crt-31 <126034653+crt-31@users.noreply.github.com> Date: Tue, 19 Dec 2023 02:42:41 -0800 Subject: [PATCH] Make tests pass on Windows (#1529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make tests pass on Windows Bunch of changes. Here's some highlights: Code Changes - Refactored ScalacWorker to abstract common code between Scala 2 & 3 - Refactored dependency-analyzer plugin javaargs to use ‘;’ as a delimiter instead of ‘:’ - Renamed a couple function names containing rpathlocation to rlocationpath - added lineEndings=preserve to scalafmt.conf - added .bazelrc to enable needed flags for Windows Test Changes - disable running tests for features not-supported on Windows - make tests handle fact that sh_binary outputs .exe on Windows - Handle other Windows specifics where needed (i.e. backslash as pathseparator, crlf linendings, dependence on bash) * Fixed some minor nits --- .bazelrc | 5 + .scalafmt.conf | 1 + dt_patches/dt_patch_test.sh | 6 +- dt_patches/test_dt_patches/.bazelrc | 1 + .../test_dt_patches_user_srcjar/.bazelrc | 1 + .../multi_frameworks_toolchain/.bazelrc | 2 + .../specs2_junit_repositories/.bazelrc | 2 + scala/private/common.bzl | 8 +- scala/private/rule_impls.bzl | 4 +- .../rulesscala/exe/LauncherFileWriter.java | 10 +- src/java/io/bazel/rulesscala/scalac/BUILD | 15 +- .../rulesscala/scalac/ScalacInvoker.java | 67 +++ .../rulesscala/scalac/ScalacInvoker3.java | 48 +++ .../scalac/ScalacInvokerResults.java | 6 + .../bazel/rulesscala/scalac/ScalacWorker.java | 86 +--- .../rulesscala/scalac/ScalacWorker3.java | 398 ------------------ .../test_discovery/ArchiveEntries.scala | 3 +- ...calaTestResourcesFromLocalTargetTest.scala | 10 +- test/aspect/BUILD | 13 +- test/aspect/aspect.bzl | 13 +- test/rootpaths_binary.sh | 15 +- test/scalafmt/.scalafmt.conf | 1 + test/shell/test_examples.sh | 35 +- test/shell/test_helper.sh | 14 + test/shell/test_misc.sh | 13 +- test/shell/test_persistent_worker.sh | 7 +- test/shell/test_scala_binary.sh | 6 +- test/shell/test_scala_jacocorunner.sh | 7 +- test/shell/test_scala_library.sh | 10 +- test/shell/test_scalafmt.sh | 14 +- .../ScalaLibResourcesDuplicatedTest.scala | 3 +- ...ScalaLibResourcesFromExternalDepTest.scala | 9 +- ...alaLibResourcesFromExternalScalaTest.scala | 3 +- test_reproducibility.sh | 11 +- test_version.sh | 4 + .../DependencyAnalyzerSettings.scala | 9 +- .../dependencyanalyzer/OptionsParser.scala | 38 +- .../dependency_analyzer/src/test/BUILD | 7 + .../dependencyanalyzer/StrictDepsTest.scala | 2 +- .../UnusedDependencyCheckerTest.scala | 2 +- .../src/test/test_optionsparser.scala | 30 ++ .../io/bazel/rulesscala/utils/TestUtil.scala | 22 +- 42 files changed, 416 insertions(+), 545 deletions(-) create mode 100644 .bazelrc create mode 100644 dt_patches/test_dt_patches/.bazelrc create mode 100644 dt_patches/test_dt_patches_user_srcjar/.bazelrc create mode 100644 examples/testing/multi_frameworks_toolchain/.bazelrc create mode 100644 examples/testing/specs2_junit_repositories/.bazelrc create mode 100644 src/java/io/bazel/rulesscala/scalac/ScalacInvoker.java create mode 100644 src/java/io/bazel/rulesscala/scalac/ScalacInvoker3.java create mode 100644 src/java/io/bazel/rulesscala/scalac/ScalacInvokerResults.java delete mode 100644 src/java/io/bazel/rulesscala/scalac/ScalacWorker3.java create mode 100644 third_party/dependency_analyzer/src/test/test_optionsparser.scala diff --git a/.bazelrc b/.bazelrc new file mode 100644 index 000000000..b37b4dc55 --- /dev/null +++ b/.bazelrc @@ -0,0 +1,5 @@ +build --enable_platform_specific_config + +#Windows needs --worker_quit_after_build due to workers not being shut down when the compiler tools need to be rebuilt (resulting in 'file in use' errors). See Bazel Issue#10498. + +build:windows --worker_quit_after_build --enable_runfiles diff --git a/.scalafmt.conf b/.scalafmt.conf index dfb81c64b..79891b377 100644 --- a/.scalafmt.conf +++ b/.scalafmt.conf @@ -13,3 +13,4 @@ rewrite.rules = [ SortImports ] unindentTopLevelOperators = false +lineEndings=preserve diff --git a/dt_patches/dt_patch_test.sh b/dt_patches/dt_patch_test.sh index b0cc017df..8d14bee2f 100755 --- a/dt_patches/dt_patch_test.sh +++ b/dt_patches/dt_patch_test.sh @@ -17,6 +17,7 @@ run_test_local() { if [ $RESPONSE_CODE -eq 0 ]; then echo -e "${GREEN} Test \"$TEST_ARG\" successful ($DURATION sec) $NC" else + echo $RES echo -e "${RED} Test \"$TEST_ARG\" failed $NC ($DURATION sec) $NC" return $RESPONSE_CODE fi @@ -26,9 +27,12 @@ run_in_test_repo() { local test_command=$1 local test_repo=$2 - cd "${dir}/${test_repo}" || exit 1 + cd "${dir}/${test_repo}" || exit 1 ${test_command} RESPONSE_CODE=$? + + bazel shutdown + cd ../.. return $RESPONSE_CODE diff --git a/dt_patches/test_dt_patches/.bazelrc b/dt_patches/test_dt_patches/.bazelrc new file mode 100644 index 000000000..005efba2f --- /dev/null +++ b/dt_patches/test_dt_patches/.bazelrc @@ -0,0 +1 @@ +import ../../.bazelrc diff --git a/dt_patches/test_dt_patches_user_srcjar/.bazelrc b/dt_patches/test_dt_patches_user_srcjar/.bazelrc new file mode 100644 index 000000000..005efba2f --- /dev/null +++ b/dt_patches/test_dt_patches_user_srcjar/.bazelrc @@ -0,0 +1 @@ +import ../../.bazelrc diff --git a/examples/testing/multi_frameworks_toolchain/.bazelrc b/examples/testing/multi_frameworks_toolchain/.bazelrc new file mode 100644 index 000000000..6cd4a6699 --- /dev/null +++ b/examples/testing/multi_frameworks_toolchain/.bazelrc @@ -0,0 +1,2 @@ +build --enable_platform_specific_config +windows:build --enable_runfiles diff --git a/examples/testing/specs2_junit_repositories/.bazelrc b/examples/testing/specs2_junit_repositories/.bazelrc new file mode 100644 index 000000000..80890a133 --- /dev/null +++ b/examples/testing/specs2_junit_repositories/.bazelrc @@ -0,0 +1,2 @@ +build --enable_platform_specific_config +windows:build --enable_runfiles \ No newline at end of file diff --git a/scala/private/common.bzl b/scala/private/common.bzl index 3a90fb54e..ca0d5d7d9 100644 --- a/scala/private/common.bzl +++ b/scala/private/common.bzl @@ -136,7 +136,11 @@ def sanitize_string_for_usage(s): res_array.append("_") return "".join(res_array) -#generates rpathlocation that should be used with the rlocation() at runtime. (rpathlocations start with repo name) +#generates an rlocationpath that can be used with the rlocation() at runtime. (rlocationpath starts with repo name) +def rlocationpath_from_file(ctx, file): + return paths.normalize(ctx.workspace_name + "/" + file.short_path) + +#generates an rlocationpath that can be used with the rlocation() at runtime. (rlocationpath start with repo name) #rootpath arg expects "rootpath" format (i.e. relative to runfilesDir/workspacename). Rootpath can be obtained by $rootpath macro or File.short_path -def rpathlocation_from_rootpath(ctx, rootpath): +def rlocationpath_from_rootpath(ctx, rootpath): return paths.normalize(ctx.workspace_name + "/" + rootpath) diff --git a/scala/private/rule_impls.bzl b/scala/private/rule_impls.bzl index 3af8cab7a..1268b6109 100644 --- a/scala/private/rule_impls.bzl +++ b/scala/private/rule_impls.bzl @@ -15,7 +15,7 @@ load("@bazel_skylib//lib:paths.bzl", "paths") load("@bazel_tools//tools/jdk:toolchain_utils.bzl", "find_java_toolchain") -load(":common.bzl", "rpathlocation_from_rootpath", _collect_plugin_paths = "collect_plugin_paths") +load(":common.bzl", "rlocationpath_from_rootpath", _collect_plugin_paths = "collect_plugin_paths") load(":resources.bzl", _resource_paths = "paths") def expand_location(ctx, flags): @@ -213,7 +213,7 @@ def java_bin_windows(ctx): if paths.is_absolute(java_runtime.java_executable_runfiles_path): java_bin = java_runtime.java_executable_runfiles_path else: - java_bin = rpathlocation_from_rootpath(ctx, java_runtime.java_executable_runfiles_path) + java_bin = rlocationpath_from_rootpath(ctx, java_runtime.java_executable_runfiles_path) return java_bin diff --git a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java index b6a0e8863..9613ec5da 100644 --- a/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java +++ b/src/java/io/bazel/rulesscala/exe/LauncherFileWriter.java @@ -31,8 +31,8 @@ public static void main(String[] args) throws IOException { .addKeyValuePair("binary_type", "Java") .addKeyValuePair("workspace_name", workspaceName) .addKeyValuePair("symlink_runfiles_enabled", "0") - .addKeyValuePair("java_bin_path", javaBinPath.replace("\\", "/")) //Expects rpathlocation (i.e. with prepended repo name) - .addKeyValuePair("jar_bin_path", rpathlocation_to_rootpath( workspaceName, jarBinPath)) //Expects rootpath location + .addKeyValuePair("java_bin_path", javaBinPath.replace("\\", "/")) //Expects rlocationpath (i.e. with prepended repo name) + .addKeyValuePair("jar_bin_path", rlocationpath_to_rootpath( workspaceName, jarBinPath)) //Expects rootpath location .addKeyValuePair("java_start_class", javaStartClass) .addKeyValuePair("classpath", classpath) //Expects rootpath location .addJoinedValues("jvm_flags", "\t", jvmFlags) @@ -56,9 +56,9 @@ public static void main(String[] args) throws IOException { } } - //Bazel's java_launcher expects some fields(i.e. jar_bin_path and classpaths) to be rootpath. rpathlocation is relative to runfiledir (always prefix with repo). Rootpath is relative to runfiledir/workspacename. - static String rpathlocation_to_rootpath(String workspaceName, String rpathlocation){ - Path path = Paths.get(rpathlocation); + //Bazel's java_launcher expects some fields(i.e. jar_bin_path and classpaths) to be rootpath. rlocationpath is relative to runfiledir (always prefix with repo). Rootpath is relative to runfiledir/workspacename. + static String rlocationpath_to_rootpath(String workspaceName, String rlocationpath){ + Path path = Paths.get(rlocationpath); Path result; if(!path.startsWith(workspaceName)){ diff --git a/src/java/io/bazel/rulesscala/scalac/BUILD b/src/java/io/bazel/rulesscala/scalac/BUILD index 017b0ff2d..146610fd9 100644 --- a/src/java/io/bazel/rulesscala/scalac/BUILD +++ b/src/java/io/bazel/rulesscala/scalac/BUILD @@ -27,7 +27,7 @@ java_binary( "-source 1.8", "-target 1.8", ], - main_class = "io.bazel.rulesscala.scalac.ScalacWorker" if SCALA_MAJOR_VERSION.startswith("2") else "io.bazel.rulesscala.scalac.ScalacWorker3", + main_class = "io.bazel.rulesscala.scalac.ScalacWorker", visibility = ["//visibility:public"], deps = DEP_REPORTING_DEPS + SCALAC_DEPS, ) @@ -41,16 +41,21 @@ java_binary( "-source 1.8", "-target 1.8", ], - main_class = "io.bazel.rulesscala.scalac.ScalacWorker" if SCALA_MAJOR_VERSION.startswith("2") else "io.bazel.rulesscala.scalac.ScalacWorker3", + main_class = "io.bazel.rulesscala.scalac.ScalacWorker", visibility = ["//visibility:public"], deps = SCALAC_DEPS, ) filegroup( name = "scalac_files", - srcs = ([ + srcs = [ "ScalacWorker.java", - "ReportableMainClass.java", - ] if SCALA_MAJOR_VERSION.startswith("2") else ["ScalacWorker3.java"]), + "ScalacInvokerResults.java", + ] + ( + [ + "ScalacInvoker.java", + "ReportableMainClass.java", + ] if SCALA_MAJOR_VERSION.startswith("2") else ["ScalacInvoker3.java"] + ), visibility = ["//visibility:public"], ) diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacInvoker.java b/src/java/io/bazel/rulesscala/scalac/ScalacInvoker.java new file mode 100644 index 000000000..0de2a622c --- /dev/null +++ b/src/java/io/bazel/rulesscala/scalac/ScalacInvoker.java @@ -0,0 +1,67 @@ +package io.bazel.rulesscala.scalac; + +import io.bazel.rulesscala.scalac.reporter.DepsTrackingReporter; +import io.bazel.rulesscala.scalac.compileoptions.CompileOptions; +import java.nio.file.Paths; +import io.bazel.rulesscala.scalac.reporter.ProtoReporter; +import scala.tools.nsc.reporters.ConsoleReporter; +import java.io.IOException; +import java.util.Arrays; +import java.nio.file.Files; + +//Invokes Scala 2 compiler +class ScalacInvoker{ + + public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] compilerArgs) + throws IOException, Exception{ + + ReportableMainClass comp = new ReportableMainClass(ops); + + ScalacInvokerResults results = new ScalacInvokerResults(); + + results.startTime = System.currentTimeMillis(); + try { + comp.process(compilerArgs); + } catch (Throwable ex) { + if (ex.toString().contains("scala.reflect.internal.Types$TypeError")) { + throw new RuntimeException("Build failure with type error", ex); + } else if (ex.toString().contains("java.lang.StackOverflowError")) { + throw new RuntimeException("Build failure with StackOverflowError", ex); + } else if (isMacroException(ex)) { + throw new RuntimeException("Build failure during macro expansion", ex); + } else { + throw ex; + } + } + + results.stopTime = System.currentTimeMillis(); + + ConsoleReporter reporter = (ConsoleReporter) comp.getReporter(); + if (reporter instanceof ProtoReporter) { + ProtoReporter protoReporter = (ProtoReporter) reporter; + protoReporter.writeTo(Paths.get(ops.diagnosticsFile)); + } + + if (reporter instanceof DepsTrackingReporter) { + DepsTrackingReporter depTrackingReporter = (DepsTrackingReporter) reporter; + depTrackingReporter.prepareReport(); + depTrackingReporter.writeDiagnostics(ops.diagnosticsFile); + } + + if (reporter.hasErrors()) { + reporter.flush(); + throw new RuntimeException("Build failed"); + } + + return results; + } + + public static boolean isMacroException(Throwable ex) { + for (StackTraceElement elem : ex.getStackTrace()) { + if (elem.getMethodName().equals("macroExpand")) { + return true; + } + } + return false; + } +} diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacInvoker3.java b/src/java/io/bazel/rulesscala/scalac/ScalacInvoker3.java new file mode 100644 index 000000000..85d655750 --- /dev/null +++ b/src/java/io/bazel/rulesscala/scalac/ScalacInvoker3.java @@ -0,0 +1,48 @@ +package io.bazel.rulesscala.scalac; + +import io.bazel.rulesscala.scalac.compileoptions.CompileOptions; +import java.nio.file.Paths; +import java.nio.file.Files; +import scala.Tuple2; +import java.io.IOException; + +import dotty.tools.dotc.reporting.Reporter; +import dotty.tools.dotc.Compiler; +import dotty.tools.dotc.Driver; +import dotty.tools.dotc.core.Contexts; +import dotty.tools.io.AbstractFile; + +//Invokes Scala 3 compiler +class ScalacInvoker{ + public static ScalacInvokerResults invokeCompiler(CompileOptions ops, String[] compilerArgs) + throws IOException, Exception{ + + + ScalacInvokerResults results = new ScalacInvokerResults(); + Driver driver = new dotty.tools.dotc.Driver(); + Contexts.Context ctx = driver.initCtx().fresh(); + + Tuple2, Contexts.Context> r = driver.setup(compilerArgs, ctx).get(); + + Compiler compiler = driver.newCompiler(r._2); + + results.startTime= System.currentTimeMillis(); + + Reporter reporter = driver.doCompile(compiler, r._1, r._2); + + results.stopTime = System.currentTimeMillis(); + + Files.createFile( + Paths.get(ops.diagnosticsFile)); + Files.createFile( + Paths.get(ops.scalaDepsFile)); + + + if (reporter.hasErrors()) { +// reporter.flush(); + throw new RuntimeException("Build failed"); + } + + return results; + } +} diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacInvokerResults.java b/src/java/io/bazel/rulesscala/scalac/ScalacInvokerResults.java new file mode 100644 index 000000000..4e4459ade --- /dev/null +++ b/src/java/io/bazel/rulesscala/scalac/ScalacInvokerResults.java @@ -0,0 +1,6 @@ +package io.bazel.rulesscala.scalac; + +public class ScalacInvokerResults{ + public long startTime; //unixTime ms + public long stopTime; //unixTime ms +} diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java b/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java index 149833cf8..57bc9fb55 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacWorker.java @@ -1,12 +1,9 @@ package io.bazel.rulesscala.scalac; import static java.io.File.pathSeparator; - import io.bazel.rulesscala.io_utils.StreamCopy; import io.bazel.rulesscala.jar.JarCreator; import io.bazel.rulesscala.scalac.compileoptions.CompileOptions; -import io.bazel.rulesscala.scalac.reporter.DepsTrackingReporter; -import io.bazel.rulesscala.scalac.reporter.ProtoReporter; import io.bazel.rulesscala.worker.Worker; import java.io.File; import java.io.FileOutputStream; @@ -19,13 +16,13 @@ import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; -import java.util.ArrayList; import java.util.Arrays; +import java.util.ArrayList; import java.util.Enumeration; import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; -import scala.tools.nsc.reporters.ConsoleReporter; +import java.util.stream.Collectors; class ScalacWorker implements Worker.Interface { @@ -174,12 +171,10 @@ private static boolean matchesFileExtensions(String fileName, String[] extension return false; } - private static String[] encodeBazelTargets(String[] targets) { - return Arrays.stream(targets).map(ScalacWorker::encodeBazelTarget).toArray(String[]::new); - } - - private static String encodeBazelTarget(String target) { - return target.replace(":", ";"); + private static String encodeStringSeqPluginParam(String[] targets) { + //Encode with ';' delimiter. also support escape for ';' because ';' is a valid (albeit uncommon) label character. + return String.join(";" + , Arrays.stream(targets).map(s->s.replace(";", "\\;")).collect(Collectors.toList())); } private static boolean isModeEnabled(String mode) { @@ -209,54 +204,39 @@ private static String[] getPluginParamsFrom(CompileOptions ops) { List pluginParams = new ArrayList<>(0); if ((isModeEnabled(ops.strictDepsMode) || isModeEnabled(ops.unusedDependencyCheckerMode))) { - String currentTarget = encodeBazelTarget(ops.currentTarget); - String[] dependencyAnalyzerParams = { "-P:dependency-analyzer:strict-deps-mode:" + ops.strictDepsMode, "-P:dependency-analyzer:unused-deps-mode:" + ops.unusedDependencyCheckerMode, - "-P:dependency-analyzer:current-target:" + currentTarget, + "-P:dependency-analyzer:current-target:" + ops.currentTarget, "-P:dependency-analyzer:dependency-tracking-method:" + ops.dependencyTrackingMethod, }; pluginParams.addAll(Arrays.asList(dependencyAnalyzerParams)); if (ops.directJars.length > 0) { - pluginParams.add("-P:dependency-analyzer:direct-jars:" + String.join(":", ops.directJars)); + pluginParams.add("-P:dependency-analyzer:direct-jars:" + encodeStringSeqPluginParam(ops.directJars)); } if (ops.directTargets.length > 0) { - String[] directTargets = encodeBazelTargets(ops.directTargets); pluginParams.add( - "-P:dependency-analyzer:direct-targets:" + String.join(":", directTargets)); + "-P:dependency-analyzer:direct-targets:" + encodeStringSeqPluginParam(ops.directTargets)); } if (ops.indirectJars.length > 0) { pluginParams.add( - "-P:dependency-analyzer:indirect-jars:" + String.join(":", ops.indirectJars)); + "-P:dependency-analyzer:indirect-jars:" + encodeStringSeqPluginParam(ops.indirectJars)); } if (ops.indirectTargets.length > 0) { - String[] indirectTargets = encodeBazelTargets(ops.indirectTargets); pluginParams.add( - "-P:dependency-analyzer:indirect-targets:" + String.join(":", indirectTargets)); + "-P:dependency-analyzer:indirect-targets:" + encodeStringSeqPluginParam(ops.indirectTargets)); } if (ops.unusedDepsIgnoredTargets.length > 0) { - String[] ignoredTargets = encodeBazelTargets(ops.unusedDepsIgnoredTargets); pluginParams.add( - "-P:dependency-analyzer:unused-deps-ignored-targets:" - + String.join(":", ignoredTargets)); + "-P:dependency-analyzer:unused-deps-ignored-targets:" + encodeStringSeqPluginParam(ops.unusedDepsIgnoredTargets)); } } return pluginParams.toArray(new String[pluginParams.size()]); } - private static boolean isMacroException(Throwable ex) { - for (StackTraceElement elem : ex.getStackTrace()) { - if (elem.getMethodName().equals("macroExpand")) { - return true; - } - } - return false; - } - private static void compileScalaSources(CompileOptions ops, String[] scalaSources, Path classes) throws IOException, Exception { @@ -270,29 +250,10 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource String[] compilerArgs = merge(ops.scalaOpts, pluginArgs, constParams, pluginParams, scalaSources); - ReportableMainClass comp = new ReportableMainClass(ops); + ScalacInvokerResults compilerResults = ScalacInvoker.invokeCompiler(ops, compilerArgs); - long start = System.currentTimeMillis(); - try { - comp.process(compilerArgs); - } catch (Throwable ex) { - if (ex.toString().contains("scala.reflect.internal.Types$TypeError")) { - throw new RuntimeException("Build failure with type error", ex); - } else if (ex.toString().contains("java.lang.StackOverflowError")) { - throw new RuntimeException("Build failure with StackOverflowError", ex); - } else if (isMacroException(ex)) { - throw new RuntimeException("Build failure during macro expansion", ex); - } else { - throw ex; - } - }finally { - comp.close(); - } - - - long stop = System.currentTimeMillis(); if (ops.printCompileTime) { - System.err.println("Compiler runtime: " + (stop - start) + "ms."); + System.err.println("Compiler runtime: " + (compilerResults.stopTime - compilerResults.startTime) + "ms."); } try { @@ -300,29 +261,12 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource // If enable stats file we write the volatile string component // otherwise empty string for better remote cache performance. if (ops.enableStatsFile) { - buildTime = Long.toString(stop - start); + buildTime = Long.toString(compilerResults.stopTime - compilerResults.startTime); } Files.write(Paths.get(ops.statsfile), Arrays.asList("build_time=" + buildTime)); } catch (IOException ex) { throw new RuntimeException("Unable to write statsfile to " + ops.statsfile, ex); } - - ConsoleReporter reporter = (ConsoleReporter) comp.getReporter(); - if (reporter instanceof ProtoReporter) { - ProtoReporter protoReporter = (ProtoReporter) reporter; - protoReporter.writeTo(Paths.get(ops.diagnosticsFile)); - } - - if (reporter instanceof DepsTrackingReporter) { - DepsTrackingReporter depTrackingReporter = (DepsTrackingReporter) reporter; - depTrackingReporter.prepareReport(); - depTrackingReporter.writeDiagnostics(ops.diagnosticsFile); - } - - if (reporter.hasErrors()) { - reporter.flush(); - throw new RuntimeException("Build failed"); - } } private static void deleteRecursively(Path directory) throws IOException { diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacWorker3.java b/src/java/io/bazel/rulesscala/scalac/ScalacWorker3.java deleted file mode 100644 index 82112771c..000000000 --- a/src/java/io/bazel/rulesscala/scalac/ScalacWorker3.java +++ /dev/null @@ -1,398 +0,0 @@ -package io.bazel.rulesscala.scalac; - -import static java.io.File.pathSeparator; - -import io.bazel.rulesscala.scalac.compileoptions.CompileOptions; -import scala.Tuple2; -import io.bazel.rulesscala.io_utils.StreamCopy; -import io.bazel.rulesscala.jar.JarCreator; -import io.bazel.rulesscala.worker.Worker; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.nio.file.FileSystems; -import java.nio.file.FileVisitResult; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.nio.file.SimpleFileVisitor; -import java.nio.file.attribute.BasicFileAttributes; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Enumeration; -import java.util.List; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; - -import dotty.tools.dotc.reporting.Reporter; -import dotty.tools.dotc.Compiler; -import dotty.tools.dotc.Driver; -import dotty.tools.dotc.core.Contexts; -import dotty.tools.io.AbstractFile; - -class ScalacWorker3 implements Worker.Interface { - - private static final boolean isWindows = - System.getProperty("os.name").toLowerCase().contains("windows"); - - public static void main(String[] args) throws Exception { - Worker.workerMain(args, new ScalacWorker3()); - } - - @Override - public void work(String[] args) throws Exception { - Path tmpPath = null; - try { - CompileOptions ops = new CompileOptions(args); - - Path outputPath = FileSystems.getDefault().getPath(ops.outputName); - tmpPath = Files.createTempDirectory(outputPath.getParent(), "tmp"); - - List jarFiles = extractSourceJars(ops, outputPath.getParent()); - List scalaJarFiles = filterFilesByExtension(jarFiles, ".scala"); - List javaJarFiles = filterFilesByExtension(jarFiles, ".java"); - - if (!ops.expectJavaOutput && ops.javaFiles.length != 0) { - throw new RuntimeException("Cannot have java source files when no expected java output"); - } - - if (!ops.expectJavaOutput && !javaJarFiles.isEmpty()) { - throw new RuntimeException( - "Found java files in source jars but expect Java output is set to false"); - } - - String[] scalaSources = collectSrcJarSources(ops.files, scalaJarFiles, javaJarFiles); - - String[] javaSources = appendToString(ops.javaFiles, javaJarFiles); - if (scalaSources.length == 0 && javaSources.length == 0) { - throw new RuntimeException("Must have input files from either source jars or local files."); - } - - /** - * Compile scala sources if available (if there are none, we will simply compile java - * sources). - */ - if (scalaSources.length > 0) { - compileScalaSources(ops, scalaSources, tmpPath); - } - - /** Copy the resources */ - copyResources(ops.resourceSources, ops.resourceTargets, tmpPath); - - /** Extract and copy resources from resource jars */ - copyResourceJars(ops.resourceJars, tmpPath); - - /** Copy classpath resources to root of jar */ - copyClasspathResourcesToRoot(ops.classpathResourceFiles, tmpPath); - - /** Now build the output jar */ - String[] jarCreatorArgs = { - "-m", ops.manifestPath, "-t", ops.stampLabel, outputPath.toString(), tmpPath.toString() - }; - JarCreator.main(jarCreatorArgs); - } finally { - removeTmp(tmpPath); - } - } - - private static String[] collectSrcJarSources( - String[] files, List scalaJarFiles, List javaJarFiles) { - String[] scalaSources = appendToString(files, scalaJarFiles); - return appendToString(scalaSources, javaJarFiles); - } - - private static List filterFilesByExtension(List files, String extension) { - List filtered = new ArrayList(); - for (File f : files) { - if (f.toString().endsWith(extension)) { - filtered.add(f); - } - } - return filtered; - } - - private static final String[] sourceExtensions = {".scala", ".java"}; - - private static List extractSourceJars(CompileOptions opts, Path tmpParent) - throws IOException { - List sourceFiles = new ArrayList(); - - for (String jarPath : opts.sourceJars) { - if (jarPath.length() > 0) { - Path tmpPath = Files.createTempDirectory(tmpParent, "tmp"); - sourceFiles.addAll(extractJar(jarPath, tmpPath.toString(), sourceExtensions)); - } - } - - return sourceFiles; - } - - private static List extractJar(String jarPath, String outputFolder, String[] extensions) - throws IOException { - - List outputPaths = new ArrayList<>(); - JarFile jar = new JarFile(jarPath); - Enumeration e = jar.entries(); - while (e.hasMoreElements()) { - JarEntry file = e.nextElement(); - String thisFileName = file.getName(); - // we don't bother to extract non-scala/java sources (skip manifest) - if (extensions != null && !matchesFileExtensions(thisFileName, extensions)) { - continue; - } - File f = new File(outputFolder + File.separator + file.getName()); - - if (file.isDirectory()) { // if it's a directory, create it - f.mkdirs(); - continue; - } - - File parent = f.getParentFile(); - parent.mkdirs(); - outputPaths.add(f); - - try (InputStream is = jar.getInputStream(file); - OutputStream fos = new FileOutputStream(f)) { - StreamCopy.copy(is, fos); - } - } - return outputPaths; - } - - private static boolean matchesFileExtensions(String fileName, String[] extensions) { - for (String e : extensions) { - if (fileName.endsWith(e)) { - return true; - } - } - return false; - } - - private static String[] encodeBazelTargets(String[] targets) { - return Arrays.stream(targets).map(ScalacWorker3::encodeBazelTarget).toArray(String[]::new); - } - - private static String encodeBazelTarget(String target) { - return target.replace(":", ";"); - } - - private static boolean isModeEnabled(String mode) { - return !"off".equals(mode); - } - - public static String[] buildPluginArgs(String[] pluginElements) { - int numPlugins = 0; - for (int i = 0; i < pluginElements.length; i++) { - if (pluginElements[i].length() > 0) { - numPlugins += 1; - } - } - - String[] result = new String[numPlugins]; - int idx = 0; - for (int i = 0; i < pluginElements.length; i++) { - if (pluginElements[i].length() > 0) { - result[idx] = "-Xplugin:" + pluginElements[i]; - idx += 1; - } - } - return result; - } - - private static String[] getPluginParamsFrom(CompileOptions ops) { - List pluginParams = new ArrayList<>(0); - - if (isModeEnabled(ops.strictDepsMode) || isModeEnabled(ops.unusedDependencyCheckerMode)) { - String currentTarget = encodeBazelTarget(ops.currentTarget); - - String[] dependencyAnalyzerParams = { - "-P:dependency-analyzer:strict-deps-mode:" + ops.strictDepsMode, - "-P:dependency-analyzer:unused-deps-mode:" + ops.unusedDependencyCheckerMode, - "-P:dependency-analyzer:current-target:" + currentTarget, - "-P:dependency-analyzer:dependency-tracking-method:" + ops.dependencyTrackingMethod, - }; - - pluginParams.addAll(Arrays.asList(dependencyAnalyzerParams)); - - if (ops.directJars.length > 0) { - pluginParams.add("-P:dependency-analyzer:direct-jars:" + String.join(":", ops.directJars)); - } - if (ops.directTargets.length > 0) { - String[] directTargets = encodeBazelTargets(ops.directTargets); - pluginParams.add( - "-P:dependency-analyzer:direct-targets:" + String.join(":", directTargets)); - } - if (ops.indirectJars.length > 0) { - pluginParams.add( - "-P:dependency-analyzer:indirect-jars:" + String.join(":", ops.indirectJars)); - } - if (ops.indirectTargets.length > 0) { - String[] indirectTargets = encodeBazelTargets(ops.indirectTargets); - pluginParams.add( - "-P:dependency-analyzer:indirect-targets:" + String.join(":", indirectTargets)); - } - if (ops.unusedDepsIgnoredTargets.length > 0) { - String[] ignoredTargets = encodeBazelTargets(ops.unusedDepsIgnoredTargets); - pluginParams.add( - "-P:dependency-analyzer:unused-deps-ignored-targets:" - + String.join(":", ignoredTargets)); - } - } - - return pluginParams.toArray(new String[pluginParams.size()]); - } - - private static void compileScalaSources(CompileOptions ops, String[] scalaSources, Path tmpPath) - throws IOException { - - Driver driver = new dotty.tools.dotc.Driver(); - Contexts.Context ctx = driver.initCtx().fresh(); - - String[] pluginArgs = buildPluginArgs(ops.plugins); - String[] pluginParams = getPluginParamsFrom(ops); - - String[] constParams = { - "-classpath", String.join(pathSeparator, ops.classpath), "-d", tmpPath.toString() - }; - - String[] compilerArgs = - merge(ops.scalaOpts, pluginArgs, constParams, pluginParams, scalaSources); - - Tuple2, Contexts.Context> r = driver.setup(compilerArgs, ctx).get(); - - Compiler compiler = driver.newCompiler(r._2); - - - - long start = System.currentTimeMillis(); - - Reporter reporter = driver.doCompile(compiler, r._1, r._2); - - long stop = System.currentTimeMillis(); - if (ops.printCompileTime) { - System.err.println("Compiler runtime: " + (stop - start) + "ms."); - } - - if (reporter.hasErrors()) { - System.out.println("FAILED"); - throw new RuntimeException("Compilation failed"); - } else { - System.out.println("SUCCESS"); - } - - try { - String buildTime = ""; - // If enable stats file we write the volatile string component - // otherwise empty string for better remote cache performance. - if (ops.enableStatsFile) { - buildTime = Long.toString(stop - start); - } - Files.write( - Paths.get(ops.statsfile), Arrays.asList("build_time=" + buildTime)); - Files.createFile( - Paths.get(ops.diagnosticsFile)); - Files.createFile( - Paths.get(ops.scalaDepsFile)); - } catch (IOException ex) { - throw new RuntimeException("Unable to write statsfile to " + ops.statsfile, ex); - } - - if (reporter.hasErrors()) { -// reporter.flush(); - throw new RuntimeException("Build failed"); - } - } - - private static void removeTmp(Path tmp) throws IOException { - if (tmp != null) { - Files.walkFileTree( - tmp, - new SimpleFileVisitor() { - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) - throws IOException { - if (isWindows) { - file.toFile().setWritable(true); - } - Files.delete(file); - return FileVisitResult.CONTINUE; - } - - @Override - public FileVisitResult postVisitDirectory(Path dir, IOException exc) - throws IOException { - Files.delete(dir); - return FileVisitResult.CONTINUE; - } - }); - } - } - - private static void copyResources(String[] sources, String[] targets, Path dest) - throws IOException { - if (sources.length != targets.length) - throw new RuntimeException( - String.format( - "mismatch in resources: sources: %s targets: %s", - Arrays.toString(sources), Arrays.toString(targets))); - - for (int i = 0; i < sources.length; i++) { - Path source = Paths.get(sources[i]); - Path target = dest.resolve(targets[i]); - target.getParent().toFile().mkdirs(); - Files.copy(source, target); - } - } - - private static void copyClasspathResourcesToRoot(String[] classpathResourceFiles, Path dest) - throws IOException { - for (String s : classpathResourceFiles) { - Path source = Paths.get(s); - Path target = dest.resolve(source.getFileName()); - - if (Files.exists(target)) { - System.err.println( - "Classpath resource file " - + source.getFileName() - + " has a namespace conflict with another file: " - + target.getFileName()); - } else { - Files.copy(source, target); - } - } - } - - private static void copyResourceJars(String[] resourceJars, Path dest) throws IOException { - for (String jarPath : resourceJars) { - extractJar(jarPath, dest.toString(), null); - } - } - - private static String[] appendToString(String[] init, List rest) { - String[] tmp = new String[init.length + rest.size()]; - System.arraycopy(init, 0, tmp, 0, init.length); - int baseIdx = init.length; - for (T t : rest) { - tmp[baseIdx] = t.toString(); - baseIdx += 1; - } - return tmp; - } - - private static String[] merge(String[]... arrays) { - int totalLength = 0; - for (String[] arr : arrays) { - totalLength += arr.length; - } - - String[] result = new String[totalLength]; - int offset = 0; - for (String[] arr : arrays) { - System.arraycopy(arr, 0, result, offset, arr.length); - offset += arr.length; - } - return result; - } -} diff --git a/src/java/io/bazel/rulesscala/test_discovery/ArchiveEntries.scala b/src/java/io/bazel/rulesscala/test_discovery/ArchiveEntries.scala index 10a6696e3..fb65141f9 100644 --- a/src/java/io/bazel/rulesscala/test_discovery/ArchiveEntries.scala +++ b/src/java/io/bazel/rulesscala/test_discovery/ArchiveEntries.scala @@ -5,8 +5,9 @@ import java.util.jar.{JarEntry, JarInputStream} object ArchiveEntries { def listClassFiles(file: File): Stream[String] = { + val allEntries = if (file.isDirectory) - directoryEntries(file).map(_.stripPrefix(file.toString).stripPrefix("/")) + directoryEntries(file).map(_.stripPrefix(file.toString).stripPrefix("/").stripPrefix("\\")) else jarEntries(new JarInputStream(new FileInputStream(file))) diff --git a/test/ScalaTestResourcesFromLocalTargetTest.scala b/test/ScalaTestResourcesFromLocalTargetTest.scala index 0d3323781..f4b369ac4 100644 --- a/test/ScalaTestResourcesFromLocalTargetTest.scala +++ b/test/ScalaTestResourcesFromLocalTargetTest.scala @@ -5,7 +5,11 @@ import org.scalatest.flatspec._ class ScalaTestResourcesFromLocalTargetTest extends AnyFlatSpec { "scala_test's resources when referencing local target" should "assert that local target is not placed in bazel-out, but rather next to the packaged code" in { - assert(getClass.getResource("/bazel-out/darwin-fastbuild/bin/test/py_resource_binary") == null) - assert(getClass.getResource("/test/py_resource_binary") != null) + + val fileExt = if (isWindows) ".exe" else "" + assert(getClass.getResource("/bazel-out/darwin-fastbuild/bin/test/py_resource_binary" + fileExt) == null) + assert(getClass.getResource("/test/py_resource_binary"+ fileExt) != null) } -} \ No newline at end of file + + def isWindows = System.getProperty("os.name").toLowerCase.contains("windows") +} diff --git a/test/aspect/BUILD b/test/aspect/BUILD index fd4122f6d..4c2174cad 100644 --- a/test/aspect/BUILD +++ b/test/aspect/BUILD @@ -1,4 +1,4 @@ -load(":aspect.bzl", "aspect_test") +load(":aspect.bzl", "aspect_testscript") load( "//scala:scala.bzl", "scala_junit_test", @@ -7,8 +7,9 @@ load( "scala_test", ) -aspect_test( - name = "aspect_test", +aspect_testscript( + name = "aspect_testscript", + testonly = True, targets = [ ":scala_library", ":scala_test", @@ -17,6 +18,12 @@ aspect_test( ], ) +sh_test( + name = "aspect_test", + testonly = True, + srcs = ["aspect_testscript"], +) + scala_library(name = "scala_library") scala_test(name = "scala_test") diff --git a/test/aspect/aspect.bzl b/test/aspect/aspect.bzl index ab64c321d..906c0f7fb 100644 --- a/test/aspect/aspect.bzl +++ b/test/aspect/aspect.bzl @@ -27,7 +27,7 @@ test_aspect = aspect( implementation = _aspect_impl, ) -def _rule_impl(ctx): +def _aspect_testscript_impl(ctx): expected_deps = { "scala_library": [ "//test/aspect:scala_library", @@ -70,17 +70,18 @@ def _rule_impl(ctx): expected = ", ".join(expected), visited = ", ".join(visited), ) + + scriptFile = ctx.actions.declare_file("aspect_test.sh") ctx.actions.write( - output = ctx.outputs.executable, + output = scriptFile, content = content, ) - return [] + return [DefaultInfo(files = depset([scriptFile]))] -aspect_test = rule( - implementation = _rule_impl, +aspect_testscript = rule( + implementation = _aspect_testscript_impl, attrs = { # The targets whose dependencies we want to verify. "targets": attr.label_list(aspects = [test_aspect]), }, - test = True, ) diff --git a/test/rootpaths_binary.sh b/test/rootpaths_binary.sh index ee4579eb5..2dc4e69fd 100755 --- a/test/rootpaths_binary.sh +++ b/test/rootpaths_binary.sh @@ -3,7 +3,20 @@ set -eou pipefail content="$(cat $1)" -expected=$'test/ScalaBinary\ntest/ScalaBinary.jar' + +function is_windows() { + [[ "${OSTYPE}" =~ msys* ]] || [[ "${OSTYPE}" =~ cygwin* ]] +} + +# Windows needs .exe suffix +if is_windows; then + binary_ext=".exe" +else + binary_ext="" +fi + +expected="test/ScalaBinary${binary_ext}"$'\ntest/ScalaBinary.jar' + if [ "$content" != "$expected" ]; then echo "Unexpected rootpaths: $content" echo "$expected" diff --git a/test/scalafmt/.scalafmt.conf b/test/scalafmt/.scalafmt.conf index 46b439c07..4edb960ca 100644 --- a/test/scalafmt/.scalafmt.conf +++ b/test/scalafmt/.scalafmt.conf @@ -1 +1,2 @@ maxColumn = 40 +lineEndings=preserve diff --git a/test/shell/test_examples.sh b/test/shell/test_examples.sh index d492ea437..e1096bee4 100755 --- a/test/shell/test_examples.sh +++ b/test/shell/test_examples.sh @@ -4,36 +4,51 @@ dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) . "${dir}"/test_helper.sh runner=$(get_test_runner "${1:-local}") +function test_example(){ + local dir1=$1; + local cmd1=$2; + ( + set -e + + cd $dir1 + $cmd1 + bazel shutdown; #cleanup bazel process + ) +} + function scalatest_repositories_example() { - (cd examples/testing/scalatest_repositories; bazel test //...) + test_example examples/testing/scalatest_repositories "bazel test //..." } function specs2_junit_repositories_example() { - (cd examples/testing/specs2_junit_repositories; bazel test //...) + + test_example examples/testing/specs2_junit_repositories "bazel test //..." } function multi_framework_toolchain_example() { - (cd examples/testing/multi_frameworks_toolchain; bazel test //...) + test_example examples/testing/multi_frameworks_toolchain "bazel test //..." } function scala3_1_example() { - (cd examples/scala3; bazel build --repo_env=SCALA_VERSION=3.1.0 //...) + test_example examples/scala3 "bazel build --repo_env=SCALA_VERSION=3.1.0 //..." } function scala3_2_example() { - (cd examples/scala3; bazel build --repo_env=SCALA_VERSION=3.2.1 //...) + test_example examples/scala3 "bazel build --repo_env=SCALA_VERSION=3.2.1 //..." } function scala3_3_example() { - (cd examples/scala3; bazel build --repo_env=SCALA_VERSION=3.3.0 //...) + test_example examples/scala3 "bazel build --repo_env=SCALA_VERSION=3.3.0 //..." } function semanticdb_example() { - set -e - ( cd examples/semanticdb; - bazel build //... --aspects aspect.bzl%semanticdb_info_aspect --output_groups=json_output_file; + + function build_semanticdb_example(){ + bazel build //... --aspects aspect.bzl%semanticdb_info_aspect --output_groups=json_output_file bazel build //... - ) + } + + test_example examples/semanticdb build_semanticdb_example } $runner scalatest_repositories_example diff --git a/test/shell/test_helper.sh b/test/shell/test_helper.sh index 8bd3c49db..36023a265 100755 --- a/test/shell/test_helper.sh +++ b/test/shell/test_helper.sh @@ -2,6 +2,14 @@ # # Test helper functions for rules_scala integration tests. +function is_windows() { + [[ "${OSTYPE}" =~ msys* ]] || [[ "${OSTYPE}" =~ cygwin* ]] +} + +function is_macos() { + [[ "${OSTYPE}" =~ darwin* ]] +} + action_should_fail() { # runs the tests locally set +e @@ -28,12 +36,14 @@ test_expect_failure_with_message() { echo ${output} | grep "$expected_message" if [ $? -ne 0 ]; then + echo ${output} echo "'bazel test ${test_command}' should have logged \"${expected_message}\"." exit 1 fi if [ "${additional_expected_message}" != "" ]; then echo ${output} | grep "$additional_expected_message" if [ $? -ne 0 ]; then + echo ${output} echo "'bazel test ${test_command}' should have logged \"${additional_expected_message}\"." exit 1 fi @@ -51,9 +61,11 @@ action_should_fail_with_message() { echo $RES | grep -- "$MSG" GREP_RES=$? if [ $RESPONSE_CODE -eq 0 ]; then + echo $RES echo -e "${RED} \"bazel $TEST_ARG\" should have failed but passed. $NC" exit 1 elif [ $GREP_RES -ne 0 ]; then + echo $RES echo -e "${RED} \"bazel $TEST_ARG\" should have failed with message \"$MSG\" but did not. $NC" exit 1 else @@ -89,12 +101,14 @@ test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message() { echo ${output} | grep "$expected_message" if [ $? -ne 0 ]; then + echo ${output} echo "'bazel build ${test_target}' should have logged \"${expected_message}\"." exit 1 fi if [ "${additional_expected_message}" != "" ]; then echo ${output} | grep "$additional_expected_message" if [ $? -ne 0 ]; then + echo ${output} echo "'bazel build ${test_target}' should have logged \"${additional_expected_message}\"." exit 1 fi diff --git a/test/shell/test_misc.sh b/test/shell/test_misc.sh index 714c87991..5447d3d8e 100755 --- a/test/shell/test_misc.sh +++ b/test/shell/test_misc.sh @@ -50,7 +50,10 @@ test_transitive_deps() { } test_repl() { - bazel build $(bazel query 'kind(scala_repl, //test/...)') + local query_results=$(bazel query 'kind(scala_repl, //test/...)') + #local query_results="${query_results//$'\r'}" #make sure \r is removed so bash can parse args correctly on windows + + bazel build "${query_results//$'\r'}" #make sure \r is removed so bash can parse args correctly on windows echo "import scalarules.test._; HelloLib.printMessage(\"foo\")" | bazel-bin/test/HelloLibRepl -Xnojline | grep "foo java" && echo "import scalarules.test._; TestUtil.foo" | bazel-bin/test/HelloLibTestRepl -Xnojline | grep "bar" && echo "import scalarules.test._; ScalaLibBinary.main(Array())" | bazel-bin/test/ScalaLibBinaryRepl -Xnojline | grep "A hui hou" && @@ -111,7 +114,7 @@ test_multi_service_manifest() { meta_file='META-INF/services/org.apache.beam.sdk.io.FileSystemRegistrar' bazel build test:$deploy_jar unzip -p bazel-bin/test/$deploy_jar $meta_file > service_manifest.txt - diff service_manifest.txt test/example_jars/expected_service_manifest.txt + diff --strip-trailing-cr service_manifest.txt test/example_jars/expected_service_manifest.txt RESPONSE_CODE=$? rm service_manifest.txt exit $RESPONSE_CODE @@ -133,5 +136,9 @@ $runner test_benchmark_jmh $runner test_benchmark_jmh_failure $runner scala_test_test_filters $runner test_multi_service_manifest -$runner test_override_javabin + +if ! is_windows; then + #javabin only affects wrapper scripts for linux + $runner test_override_javabin +fi $runner xmllint_test diff --git a/test/shell/test_persistent_worker.sh b/test/shell/test_persistent_worker.sh index 0fb604b47..8d0789de6 100755 --- a/test/shell/test_persistent_worker.sh +++ b/test/shell/test_persistent_worker.sh @@ -5,7 +5,12 @@ dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) . "${dir}"/test_helper.sh runner=$(get_test_runner "${1:-local}") -PERSISTENT_WORKER_FLAGS="--strategy=Scalac=worker --worker_sandboxing" +if is_windows; then + #Bazel sandboxing is not currently implemented in windows + PERSISTENT_WORKER_FLAGS="--strategy=Scalac=worker" +else + PERSISTENT_WORKER_FLAGS="--strategy=Scalac=worker --worker_sandboxing" +fi check_persistent_worker_failure() { command=$1 diff --git a/test/shell/test_scala_binary.sh b/test/shell/test_scala_binary.sh index d8036e938..b562008f3 100755 --- a/test/shell/test_scala_binary.sh +++ b/test/shell/test_scala_binary.sh @@ -40,4 +40,8 @@ test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_p $runner test_scala_binary_expect_failure_on_missing_direct_deps $runner test_scala_binary_expect_failure_on_missing_direct_deps_located_in_dependency_which_is_scala_binary -$runner test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_performance + +if ! is_windows; then + #rules_scala doesn't support argfiles on windows yet + $runner test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_performance +fi diff --git a/test/shell/test_scala_jacocorunner.sh b/test/shell/test_scala_jacocorunner.sh index 434d8dd67..77031652c 100755 --- a/test/shell/test_scala_jacocorunner.sh +++ b/test/shell/test_scala_jacocorunner.sh @@ -12,5 +12,8 @@ test_scala_jacocorunner_from_scala_toolchain_fails() { action_should_fail coverage --extra_toolchains="//test_expect_failure/scala_test_jacocorunner:failing_scala_toolchain" //test_expect_failure/scala_test_jacocorunner:empty_test } -$runner test_scala_jacocorunner_from_scala_toolchain_passes -$runner test_scala_jacocorunner_from_scala_toolchain_fails +#Jacocoa support not implemented for windows just yet +if ! is_windows; then + $runner test_scala_jacocorunner_from_scala_toolchain_passes + $runner test_scala_jacocorunner_from_scala_toolchain_fails +fi diff --git a/test/shell/test_scala_library.sh b/test/shell/test_scala_library.sh index dcf8d9821..ec43e1e7d 100755 --- a/test/shell/test_scala_library.sh +++ b/test/shell/test_scala_library.sh @@ -98,8 +98,14 @@ test_scala_library_expect_failure_on_missing_direct_deps_warn_mode_java() { } test_scala_library_expect_failure_on_missing_direct_deps_off_mode() { - expected_message="test_expect_failure/missing_direct_deps/internal_deps/A.scala:[0-9+]: error: not found: value C" - test_target='test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_user' + #scalac outputs backslashes on windows (triple slash needed for the grep) + if is_windows; then + local expected_message="test_expect_failure\\\missing_direct_deps\\\internal_deps\\\A.scala:[0-9+]: error: not found: value C" + + else + local expected_message="test_expect_failure/missing_direct_deps/internal_deps/A.scala:[0-9+]: error: not found: value C" + fi + local test_target='test_expect_failure/missing_direct_deps/internal_deps:transitive_dependency_user' test_expect_failure_or_warning_on_missing_direct_deps_with_expected_message "${expected_message}" ${test_target} "--extra_toolchains=//test/toolchains:high_level_direct_deps" } diff --git a/test/shell/test_scalafmt.sh b/test/shell/test_scalafmt.sh index 03db1a895..24e138718 100755 --- a/test/shell/test_scalafmt.sh +++ b/test/shell/test_scalafmt.sh @@ -24,13 +24,20 @@ run_formatting() { RULE_TYPE=$1 FILENAME=$2 - bazel run //test/scalafmt:formatted-$RULE_TYPE.format-test + #on windows scalafmt targets need to be run using bash. + #TODO: improve the scalafmt funcitonality so we don't need to use the run_under mechanism + local run_under = "" + if is_windows; then + run_under="--run_under=bash" + fi + + bazel run //test/scalafmt:formatted-$RULE_TYPE.format-test $run_under if [ $? -ne 0 ]; then echo -e "${RED} formatted-$RULE_TYPE.format-test should be a formatted target. $NC" exit 1 fi - bazel run //test/scalafmt:unformatted-$RULE_TYPE.format-test + bazel run //test/scalafmt:unformatted-$RULE_TYPE.format-test $run_under if [ $? -eq 0 ]; then echo -e "${RED} unformatted-$RULE_TYPE.format-test should be an unformatted target. $NC" exit 1 @@ -38,7 +45,8 @@ run_formatting() { backup_unformatted $FILE_PATH $FILENAME # format unformatted*.scala - bazel run //test/scalafmt:unformatted-$RULE_TYPE.format + + bazel run //test/scalafmt:unformatted-$RULE_TYPE.format $run_under if [ $? -ne 0 ]; then echo -e "${RED} unformatted-$RULE_TYPE.format should run formatting. $NC" restore_unformatted_before_exit $FILE_PATH $FILENAME diff --git a/test/src/main/scala/scalarules/test/duplicated_resources/child/ScalaLibResourcesDuplicatedTest.scala b/test/src/main/scala/scalarules/test/duplicated_resources/child/ScalaLibResourcesDuplicatedTest.scala index 2dc367021..7af8791af 100644 --- a/test/src/main/scala/scalarules/test/duplicated_resources/child/ScalaLibResourcesDuplicatedTest.scala +++ b/test/src/main/scala/scalarules/test/duplicated_resources/child/ScalaLibResourcesDuplicatedTest.scala @@ -5,7 +5,8 @@ import org.scalatest.funsuite._ class ScalaLibResourcesDuplicatedTest extends AnyFunSuite { test("Scala library depends on resource + deps that contains same name resources, have higher priority on this target's resource.") { - assert(get("/resource.txt") === "I am a text resource from child!\n") + //Using platform dependent newline (%n) + assert(get("/resource.txt") === String.format("I am a text resource from child!%n")) } private def get(s: String): String = diff --git a/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalDepTest.scala b/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalDepTest.scala index 870c52c78..10e4564f0 100644 --- a/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalDepTest.scala +++ b/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalDepTest.scala @@ -6,11 +6,14 @@ class ScalaLibResourcesFromExternalDepTest extends SpecWithJUnit { "Scala library depending on resources from external resource-only jar" >> { "allow to load resources" >> { - get("/external/test_new_local_repo/resource.txt") must beEqualTo("A resource\n") + + val expectedString = String.format("A resource%n"); //Using platform dependent newline (%n) + get("/external/test_new_local_repo/resource.txt") must beEqualTo(expectedString) + } } - private def get(s: String): String = + private def get(s: String): String ={ scala.io.Source.fromInputStream(getClass.getResourceAsStream(s)).mkString - + } } diff --git a/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalScalaTest.scala b/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalScalaTest.scala index c9cc74bd1..99a9e8e7f 100644 --- a/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalScalaTest.scala +++ b/test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalScalaTest.scala @@ -5,7 +5,8 @@ import org.scalatest.funsuite._ class ScalaLibResourcesFromExternalScalaTest extends AnyFunSuite { test("Scala library depending on resources from external resource-only jar should allow to load resources") { - assert(get("/external/test_new_local_repo/resource.txt") === "A resource\n") + val expectedString = String.format("A resource%n"); //Using platform dependent newline (%n) + assert(get("/external/test_new_local_repo/resource.txt") === expectedString) } private def get(s: String): String = diff --git a/test_reproducibility.sh b/test_reproducibility.sh index a72707c31..bde425077 100755 --- a/test_reproducibility.sh +++ b/test_reproducibility.sh @@ -21,13 +21,21 @@ non_deploy_jar_md5_sum() { } test_build_is_identical() { + bazel clean #ensure we are starting from square one bazel build test/... # Also build instrumented jars. bazel build --collect_code_coverage -- //test/coverage/... non_deploy_jar_md5_sum > hash1 bazel clean sleep 10 # to make sure that if timestamps slip in we get different ones - random_dir=/tmp/$RANDOM + + local random_dir=$(mktemp -d -t test_repro-XXXXXXXXXX) + + if is_windows; then + #need true os path to pass to Bazel's cmdline option + random_dir=$(cygpath -w $random_dir) + fi + bazel build --disk_cache $random_dir test/... bazel build --disk_cache $random_dir --collect_code_coverage -- //test/coverage/... non_deploy_jar_md5_sum > hash2 @@ -37,6 +45,7 @@ test_build_is_identical() { test_dir=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )/test/shell # shellcheck source=./test_runner.sh . "${test_dir}"/test_runner.sh +. "${test_dir}"/test_helper.sh runner=$(get_test_runner "${1:-local}") diff --git a/test_version.sh b/test_version.sh index 7b9c6b807..549e3cdaf 100755 --- a/test_version.sh +++ b/test_version.sh @@ -44,12 +44,16 @@ run_in_test_repo() { sed \ -e "s%\${twitter_scrooge_repositories}%$TWITTER_SCROOGE_REPOSITORIES%" \ WORKSPACE.template >> $NEW_TEST_DIR/WORKSPACE + cp ../.bazelrc $NEW_TEST_DIR/.bazelrc cd $NEW_TEST_DIR ${test_command} RESPONSE_CODE=$? + #make sure bazel still not running for this workspace + bazel shutdown + cd .. rm -rf $NEW_TEST_DIR diff --git a/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerSettings.scala b/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerSettings.scala index f1af30b62..9523d3c3a 100644 --- a/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerSettings.scala +++ b/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerSettings.scala @@ -62,15 +62,11 @@ object DependencyAnalyzerSettings { val optionsParser = OptionsParser.create(options, error) - def decodeTarget(target: String): String = { - target.replace(";", ":") - } - def parseTargetSet(prefix: String): TargetSet = { new TargetSet( prefix = prefix, jarsSeq = optionsParser.takeStringSeqOpt(s"$prefix-jars").getOrElse(Seq.empty), - targetsSeq = optionsParser.takeStringSeqOpt(s"$prefix-targets").map(_.map(decodeTarget)).getOrElse(Seq.empty) + targetsSeq = optionsParser.takeStringSeqOpt(s"$prefix-targets").getOrElse(Seq.empty) ) } @@ -88,7 +84,7 @@ object DependencyAnalyzerSettings { val settings = DependencyAnalyzerSettings( - currentTarget = decodeTarget(optionsParser.takeString("current-target")), + currentTarget = optionsParser.takeString("current-target"), dependencyTrackingMethod = DependencyTrackingMethod .parse(optionsParser.takeString("dependency-tracking-method")) @@ -104,7 +100,6 @@ object DependencyAnalyzerSettings { optionsParser .takeStringSeqOpt(s"unused-deps-ignored-targets") .getOrElse(Seq.empty) - .map(decodeTarget) .toSet ) optionsParser.failOnUnparsedOptions() diff --git a/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/OptionsParser.scala b/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/OptionsParser.scala index 4e948de7e..ae4ce9501 100644 --- a/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/OptionsParser.scala +++ b/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer/OptionsParser.scala @@ -22,6 +22,42 @@ object OptionsParser { new OptionsParser(error = error, options = optionsMap) } + + def decodeStringSeqOpt(targetsStr: String): Seq[String] = { + //Lists of items are demlimited by ';' allowing for escaped ';' (since ; is valid in a bazel label) + + def extractAndAppendToken(tokens:List[String], str:String, startIdx:Int, endIdx:Int) : List[String] = { + val token = str.substring(startIdx, endIdx).replace("\\", "") + if(!token.isEmpty()){ + return tokens :+ token; + } + return tokens; + } + + @annotation.tailrec + def tokenize(tokens:List[String], targetsStr:String, currTokenStartIdx:Int, currIdx:Int, isEscaped:Boolean) : Seq[String] = { + + if(currIdx >= targetsStr.size) + return tokens; + + val currChar = targetsStr.charAt(currIdx) + + val isNextEscaped = if (!isEscaped) (currChar == '\\') else false; + + if(!isEscaped && currChar == ';'){ + val updatedTokens = extractAndAppendToken(tokens, targetsStr, currTokenStartIdx, currIdx); + + return tokenize(updatedTokens, targetsStr, currIdx + 1, currIdx + 1, isNextEscaped) + + }else if(currIdx == targetsStr.size-1){ + return extractAndAppendToken(tokens, targetsStr, currTokenStartIdx, targetsStr.size); + }else{ + return tokenize(tokens, targetsStr, currTokenStartIdx, currIdx+1, isNextEscaped) + } + } + + tokenize(List[String](), targetsStr, 0, 0, false); + } } class OptionsParser private( @@ -46,6 +82,6 @@ class OptionsParser private( } def takeStringSeqOpt(key: String): Option[Seq[String]] = { - takeStringOpt(key).map(_.split(":").toSeq) + takeStringOpt(key).map(OptionsParser.decodeStringSeqOpt) } } diff --git a/third_party/dependency_analyzer/src/test/BUILD b/third_party/dependency_analyzer/src/test/BUILD index 000b77b5c..810de981a 100644 --- a/third_party/dependency_analyzer/src/test/BUILD +++ b/third_party/dependency_analyzer/src/test/BUILD @@ -1,3 +1,10 @@ load(":analyzer_test.bzl", "tests") +load("//scala:scala.bzl", "scala_test") tests() + +scala_test( + name = "test_optionsparser", + srcs = ["test_optionsparser.scala"], + deps = ["//third_party/dependency_analyzer/src/main:dependency_analyzer"], +) diff --git a/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/StrictDepsTest.scala b/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/StrictDepsTest.scala index c1baa4cde..b10bf152e 100644 --- a/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/StrictDepsTest.scala +++ b/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/StrictDepsTest.scala @@ -36,7 +36,7 @@ class StrictDepsTest extends AnyFunSuite { val commonsTarget = "//commons:Target" - val indirect = List(apacheCommonsClasspath -> encodeLabel(commonsTarget)) + val indirect = List(apacheCommonsClasspath -> commonsTarget) compileWithDependencyAnalyzer(testCode, withIndirect = indirect).expectErrorOn(commonsTarget) } diff --git a/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/UnusedDependencyCheckerTest.scala b/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/UnusedDependencyCheckerTest.scala index 700b07f6a..648a13a6e 100644 --- a/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/UnusedDependencyCheckerTest.scala +++ b/third_party/dependency_analyzer/src/test/io/bazel/rulesscala/dependencyanalyzer/UnusedDependencyCheckerTest.scala @@ -32,7 +32,7 @@ class UnusedDependencyCheckerTest extends AnyFunSuite { val commonsTarget = "//commons:Target" - val direct = List(apacheCommonsClasspath -> encodeLabel(commonsTarget)) + val direct = List(apacheCommonsClasspath -> commonsTarget) val errorMesssages = compileWithUnusedDependencyChecker(testCode, withDirect = direct) assert(errorMesssages.exists { msg => diff --git a/third_party/dependency_analyzer/src/test/test_optionsparser.scala b/third_party/dependency_analyzer/src/test/test_optionsparser.scala new file mode 100644 index 000000000..43257230d --- /dev/null +++ b/third_party/dependency_analyzer/src/test/test_optionsparser.scala @@ -0,0 +1,30 @@ +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.must.Matchers +import io.bazel.rulesscala.dependencyanalyzer.OptionsParser; + +class TargetSetParserTest extends AnyFlatSpec with Matchers { + + //Test parsing of ';' delimited string with escapes + "OptionsParser" should "handle ; as escaped delimiter" in { + + val result = OptionsParser.decodeStringSeqOpt(";//Pkg:tgt1\\;23;f;;;g;"); + val expectedList = Seq[String]("//Pkg:tgt1;23", "f", "g"); + assert(result === expectedList); + } + + //Test parsing of ';' delimited string with escapes + "OptionsParser" should "handle ; as escaped delimiter (end in escape)" in { + + val result = OptionsParser.decodeStringSeqOpt(";Tgt1;tgt2\\;"); + val expectedList = Seq[String]("Tgt1", "tgt2;"); + assert(result === expectedList); + } + + "OptionsParser" should "handle empty string" in { + + val result = OptionsParser.decodeStringSeqOpt(""); + val expectedList = Seq[String](); + assert(result === expectedList); + + } +} diff --git a/third_party/utils/src/test/io/bazel/rulesscala/utils/TestUtil.scala b/third_party/utils/src/test/io/bazel/rulesscala/utils/TestUtil.scala index 7b1f48df5..8f16951e2 100644 --- a/third_party/utils/src/test/io/bazel/rulesscala/utils/TestUtil.scala +++ b/third_party/utils/src/test/io/bazel/rulesscala/utils/TestUtil.scala @@ -17,9 +17,17 @@ import io.bazel.rulesscala.dependencyanalyzer.DependencyTrackingMethod object TestUtil { final val defaultTarget = "//..." + val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("windows") + + //Backslashes used as escape, so paths should use forward slashes to simulate the scalacworker. + private def normalizePath(s:String): String = { + if (isWindows) s.replace('\\', '/') else s + } + private def constructPluginParam(pluginName: String)(name: String, values: Iterable[String]): String = { + //Using ';' as a param value delimiter, and then need to escape any ';' thats in a param's value if (values.isEmpty) "" - else s"-P:$pluginName:$name:${values.mkString(":")}" + else s"-P:$pluginName:$name:${values.map(s=>s.replace(";", "\\;")).mkString(";")}" } private lazy val toolboxPluginOptions: String = { @@ -48,9 +56,9 @@ object TestUtil { "current-target" -> Seq(TestUtil.defaultTarget), "unused-deps-mode" -> (if (params.unusedDeps) { Seq("error") } else { Seq() }), "strict-deps-mode" -> (if (params.strictDeps) { Seq("error") } else { Seq() }), - "direct-jars" -> params.directJars, + "direct-jars" -> params.directJars.map(normalizePath), "direct-targets" -> params.directTargets, - "indirect-jars" -> params.indirectJars, + "indirect-jars" -> params.indirectJars.map(normalizePath), "indirect-targets" -> params.indirectTargets ) val constructParam = TestUtil.constructPluginParam("dependency-analyzer") _ @@ -71,7 +79,7 @@ object TestUtil { if (classpathEntries.isEmpty) { "" } else { - s"-classpath ${classpathEntries.mkString(":")}" + s"-classpath ${classpathEntries.map(normalizePath).mkString(java.io.File.pathSeparator)}" } } @@ -148,8 +156,4 @@ object TestUtil { val libPath = Paths.get(baseDir, jar).toAbsolutePath libPath.toString } - - def decodeLabel(targetLabel: String): String = targetLabel.replace(";", ":") - - def encodeLabel(targetLabel: String): String = targetLabel.replace(":", ";") -} \ No newline at end of file +}