From 1662f32c72a4c3869fda52493bf08003ca890ad4 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 09:54:51 +0200 Subject: [PATCH 01/12] fix spotbugs warnings in integration tests --- .../org/lflang/tests/lsp/ErrorInserter.java | 23 ++++++++++++------- .../java/org/lflang/tests/lsp/LspTests.java | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/core/src/integrationTest/java/org/lflang/tests/lsp/ErrorInserter.java b/core/src/integrationTest/java/org/lflang/tests/lsp/ErrorInserter.java index a1948821eb..13b10156e2 100644 --- a/core/src/integrationTest/java/org/lflang/tests/lsp/ErrorInserter.java +++ b/core/src/integrationTest/java/org/lflang/tests/lsp/ErrorInserter.java @@ -4,14 +4,11 @@ import java.io.Closeable; import java.io.IOException; import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.ListIterator; -import java.util.Random; +import java.nio.file.Paths; +import java.util.*; import java.util.function.BiFunction; import java.util.function.BiPredicate; import java.util.function.Function; @@ -125,7 +122,7 @@ public void write() throws IOException { if (!src.toFile().renameTo(swapFile(src).toFile())) { throw new IOException("Failed to create a swap file."); } - try (PrintWriter writer = new PrintWriter(src.toFile())) { + try (PrintWriter writer = new PrintWriter(src.toFile(), StandardCharsets.UTF_8)) { lines.forEach(writer::println); } } @@ -233,7 +230,13 @@ private void alter(BiFunction, String, Boolean> alterer) { /** Return the swap file associated with {@code f}. */ private static Path swapFile(Path p) { - return p.getParent().resolve("." + p.getFileName() + ".swp"); + final var parent = p.getParent(); + final var swpName = "." + p.getFileName() + ".swp"; + if (parent == null) { + return Paths.get(swpName); + } else { + return parent.resolve(swpName); + } } } @@ -265,6 +268,10 @@ public boolean hasNext() { @Override public T next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + T ret = current.item; current = current.previous; return ret; diff --git a/core/src/integrationTest/java/org/lflang/tests/lsp/LspTests.java b/core/src/integrationTest/java/org/lflang/tests/lsp/LspTests.java index 9270d9ea82..964963bd0e 100644 --- a/core/src/integrationTest/java/org/lflang/tests/lsp/LspTests.java +++ b/core/src/integrationTest/java/org/lflang/tests/lsp/LspTests.java @@ -91,7 +91,7 @@ void typescriptValidationTest() throws IOException { */ private void targetLanguageValidationTest(Target target, ErrorInserter.Builder builder) throws IOException { - long seed = new Random().nextLong(); + long seed = System.nanoTime(); System.out.printf( "Running validation tests for %s with random seed %d.%n", target.getDisplayName(), seed); Random random = new Random(seed); From 083f288fec12c08395439d5b8c3bab2fdce1e777 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 10:45:06 +0200 Subject: [PATCH 02/12] make TestError immutable --- .../java/org/lflang/tests/LFTest.java | 4 ++-- .../java/org/lflang/tests/TestError.java | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/core/src/testFixtures/java/org/lflang/tests/LFTest.java b/core/src/testFixtures/java/org/lflang/tests/LFTest.java index ee67dcbb96..702f8cf4af 100644 --- a/core/src/testFixtures/java/org/lflang/tests/LFTest.java +++ b/core/src/testFixtures/java/org/lflang/tests/LFTest.java @@ -165,9 +165,9 @@ public void handleTestError(TestError e) { if (e.getMessage() != null) { issues.append(e.getMessage()); } - if (e.getException() != null) { + if (e.causeIsException()) { issues.append(System.lineSeparator()); - issues.append(TestBase.stackTraceToString(e.getException())); + issues.append(e.getOriginalStackTrace()); } } diff --git a/core/src/testFixtures/java/org/lflang/tests/TestError.java b/core/src/testFixtures/java/org/lflang/tests/TestError.java index efb7d50048..fa22da447e 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestError.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestError.java @@ -5,12 +5,14 @@ /// Indicates an error during test execution public class TestError extends Exception { - private Throwable exception; - private Result result; + private final String stackTrace; + private final Result result; public TestError(String errorMessage, Result result, Throwable exception) { super(errorMessage); - this.exception = exception; + assert result != null; + + this.stackTrace = exception == null ? null : TestBase.stackTraceToString(exception); this.result = result; } @@ -26,7 +28,13 @@ public Result getResult() { return result; } - public Throwable getException() { - return exception; + /// Return true, if the TestError instance was created based on an exception. + public boolean causeIsException() { + return stackTrace != null; + } + + /// Retrieve the stack trace of the exception that caused the test error. + public String getOriginalStackTrace() { + return stackTrace; } } From 9ce13531e7bee266fa499e80f8cb8c0f6877c814 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 10:55:13 +0200 Subject: [PATCH 03/12] fix spotbugs warnings in cli test fixtures --- .../java/org/lflang/cli/CliToolTestFixture.java | 15 ++++++++++----- .../java/org/lflang/cli/TestUtils.java | 5 ++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cli/base/src/testFixtures/java/org/lflang/cli/CliToolTestFixture.java b/cli/base/src/testFixtures/java/org/lflang/cli/CliToolTestFixture.java index eab8f1aa9c..0950ad0098 100644 --- a/cli/base/src/testFixtures/java/org/lflang/cli/CliToolTestFixture.java +++ b/cli/base/src/testFixtures/java/org/lflang/cli/CliToolTestFixture.java @@ -31,6 +31,7 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import org.hamcrest.Matcher; import org.opentest4j.AssertionFailedError; @@ -66,7 +67,11 @@ public ExecutionResult run(Path wd, String... args) { ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayOutputStream err = new ByteArrayOutputStream(); - Io testIo = new Io(new PrintStream(err), new PrintStream(out), wd); + Io testIo = + new Io( + new PrintStream(err, false, StandardCharsets.UTF_8), + new PrintStream(out, false, StandardCharsets.UTF_8), + wd); int exitCode = testIo.fakeSystemExit(io -> runCliProgram(io, args)); return new ExecutionResult(out, err, exitCode); @@ -82,11 +87,11 @@ public ExecutionResult run(Path wd, String... args) { record ExecutionResult(ByteArrayOutputStream out, ByteArrayOutputStream err, int exitCode) { public String getOut() { - return out.toString(); + return out.toString(StandardCharsets.UTF_8); } public String getErr() { - return err.toString(); + return err.toString(StandardCharsets.UTF_8); } public void checkOk() { @@ -117,9 +122,9 @@ public void verify(ThrowingConsumer actions) { System.out.println("TEST FAILED"); System.out.println("> Return code: " + exitCode); System.out.println("> Standard output -------------------------"); - System.err.println(out.toString()); + System.err.println(out.toString(StandardCharsets.UTF_8)); System.out.println("> Standard error --------------------------"); - System.err.println(err.toString()); + System.err.println(err.toString(StandardCharsets.UTF_8)); System.out.println("> -----------------------------------------"); if (e instanceof Exception) { diff --git a/cli/base/src/testFixtures/java/org/lflang/cli/TestUtils.java b/cli/base/src/testFixtures/java/org/lflang/cli/TestUtils.java index 442742768f..aa471a9ca8 100644 --- a/cli/base/src/testFixtures/java/org/lflang/cli/TestUtils.java +++ b/cli/base/src/testFixtures/java/org/lflang/cli/TestUtils.java @@ -111,7 +111,10 @@ public TempDirBuilder file(String relativePath, String contents) throws IOExcept throw new IllegalArgumentException("Should be a relative path: " + relativePath); } Path filePath = curDir.resolve(relPath); - Files.createDirectories(filePath.getParent()); + final var parent = filePath.getParent(); + if (parent != null) { + Files.createDirectories(parent); + } Files.writeString(filePath, contents); return this; } From 468a4a5a1c17917661e45b5f4376ed0dfd0c11d0 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 13:49:48 +0200 Subject: [PATCH 04/12] simplifications --- core/src/main/java/org/lflang/Target.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/lflang/Target.java b/core/src/main/java/org/lflang/Target.java index 096788484e..8ab6af2891 100644 --- a/core/src/main/java/org/lflang/Target.java +++ b/core/src/main/java/org/lflang/Target.java @@ -469,16 +469,10 @@ public boolean supportsInheritance() { /** Return true if the target supports multiports and banks of reactors. */ public boolean supportsMultiports() { - switch (this) { - case C: - case CCPP: - case CPP: - case Python: - case Rust: - case TS: - return true; - } - return false; + return switch (this) { + case C, CCPP, CPP, Python, Rust, TS -> true; + default -> false; + }; } /** @@ -494,14 +488,11 @@ public boolean supportsParameterizedWidths() { * this target. */ public boolean supportsReactionDeclarations() { - if (this.equals(Target.C) || this.equals(Target.CPP)) return true; - else return false; + return this.equals(Target.C) || this.equals(Target.CPP); } /** * Return true if this code for this target should be built using Docker if Docker is used. - * - * @return */ public boolean buildsUsingDocker() { return switch (this) { From 352925e2b479db7e83bf217eb26b37c4fbc6dd11 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 14:23:57 +0200 Subject: [PATCH 05/12] clean up Target.java --- core/src/main/java/org/lflang/Target.java | 42 ++++------------------- 1 file changed, 6 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/lflang/Target.java b/core/src/main/java/org/lflang/Target.java index 8ab6af2891..c7aadda74f 100644 --- a/core/src/main/java/org/lflang/Target.java +++ b/core/src/main/java/org/lflang/Target.java @@ -24,17 +24,15 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import javax.annotation.concurrent.Immutable; import org.lflang.lf.TargetDecl; /** - * Enumeration of targets and their associated properties. These classes are written in Java, not - * Xtend, because the enum implementation in Xtend more primitive. It is safer to use enums rather - * than string values since it allows faulty references to be caught at compile time. Switch - * statements that take as input an enum but do not have cases for all members of the enum are also - * reported by Xtend with a warning message. + * Enumeration of targets and their associated properties. * * @author Marten Lohstroh */ +@Immutable public enum Target { C( "C", @@ -90,8 +88,6 @@ public enum Target { CPP( "Cpp", true, - "cpp", - "Cpp", Arrays.asList( // List via: https://en.cppreference.com/w/cpp/keyword "alignas", // (since C++11) @@ -194,8 +190,6 @@ public enum Target { TS( "TypeScript", false, - "ts", - "TS", Arrays.asList( // List via: https://github.com/Microsoft/TypeScript/issues/2536 // Reserved words @@ -352,8 +346,6 @@ public enum Target { Rust( "Rust", true, - "rust", - "Rust", // In our Rust implementation, the only reserved keywords // are those that are a valid expression. Others may be escaped // with the syntax r#keyword. @@ -362,17 +354,11 @@ public enum Target { /** String representation of this target. */ private final String displayName; - /** Name of package containing Kotlin classes for the target language. */ - public final String packageName; - - /** Prefix of names of Kotlin classes for the target language. */ - public final String classNamePrefix; - /** Whether or not this target requires types. */ public final boolean requiresTypes; /** Reserved words in the target language. */ - public final Set keywords; + private final Set keywords; /** An unmodifiable list of all known targets. */ public static final List ALL = List.of(Target.values()); @@ -382,26 +368,12 @@ public enum Target { * * @param displayName String representation of this target. * @param requiresTypes Types Whether this target requires type annotations or not. - * @param packageName Name of package containing Kotlin classes for the target language. - * @param classNamePrefix Prefix of names of Kotlin classes for the target language. * @param keywords List of reserved strings in the target language. */ - Target( - String displayName, - boolean requiresTypes, - String packageName, - String classNamePrefix, - Collection keywords) { + Target(String displayName, boolean requiresTypes, Collection keywords) { this.displayName = displayName; this.requiresTypes = requiresTypes; this.keywords = Collections.unmodifiableSet(new LinkedHashSet<>(keywords)); - this.packageName = packageName; - this.classNamePrefix = classNamePrefix; - } - - /** Private constructor for targets without packageName and classNamePrefix. */ - Target(String displayName, boolean requiresTypes, Collection keywords) { - this(displayName, requiresTypes, "N/A", "N/A", keywords); // FIXME: prefix } /** @@ -491,9 +463,7 @@ public boolean supportsReactionDeclarations() { return this.equals(Target.C) || this.equals(Target.CPP); } - /** - * Return true if this code for this target should be built using Docker if Docker is used. - */ + /** Return true if this code for this target should be built using Docker if Docker is used. */ public boolean buildsUsingDocker() { return switch (this) { case TS -> false; From 2616fa05647d57f405dd4465b20df20cbdec11c8 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 14:29:30 +0200 Subject: [PATCH 06/12] fix spotbugs warnings in TestBase --- .../java/org/lflang/tests/LFTest.java | 38 ++++++++++--------- .../java/org/lflang/tests/TestBase.java | 29 +------------- 2 files changed, 23 insertions(+), 44 deletions(-) diff --git a/core/src/testFixtures/java/org/lflang/tests/LFTest.java b/core/src/testFixtures/java/org/lflang/tests/LFTest.java index 702f8cf4af..fb2198d8be 100644 --- a/core/src/testFixtures/java/org/lflang/tests/LFTest.java +++ b/core/src/testFixtures/java/org/lflang/tests/LFTest.java @@ -1,11 +1,7 @@ package org.lflang.tests; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.io.Reader; +import java.io.*; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.nio.file.Paths; import org.eclipse.xtext.util.RuntimeIOException; @@ -67,11 +63,6 @@ public LFTest(LFTest test) { this(test.target, test.srcPath); } - /** Stream object for capturing standard and error output. */ - public OutputStream getOutputStream() { - return compilationLog; - } - public FileConfig getFileConfig() { return context.getFileConfig(); } @@ -84,6 +75,20 @@ public Path getSrcPath() { return srcPath; } + /** Redirect outputs for recording. */ + public void redirectOutputs() { + System.setOut(new PrintStream(compilationLog, false, StandardCharsets.UTF_8)); + System.setErr(new PrintStream(compilationLog, false, StandardCharsets.UTF_8)); + } + + /** End output redirection. */ + public void restoreOutputs() { + System.out.flush(); + System.err.flush(); + System.setOut(System.out); + System.setErr(System.err); + } + /** * Comparison implementation to allow for tests to be sorted (e.g., when added to a tree set) * based on their path (relative to the root of the test directory). @@ -148,12 +153,12 @@ public void reportErrors() { System.out.println( "Failed: " + this - + String.format(" in %.2f seconds\n", getExecutionTimeNanoseconds() / 1.0e9)); + + String.format(" in %.2f seconds%n", getExecutionTimeNanoseconds() / 1.0e9)); System.out.println( "-----------------------------------------------------------------------------"); System.out.println("Reason: " + this.result.message); printIfNotEmpty("Reported issues", this.issues.toString()); - printIfNotEmpty("Compilation output", this.compilationLog.toString()); + printIfNotEmpty("Compilation output", this.compilationLog.toString(StandardCharsets.UTF_8)); printIfNotEmpty("Execution output", this.execLog.toString()); System.out.println( "+---------------------------------------------------------------------------+"); @@ -258,14 +263,13 @@ public Thread recordStdErr(Process process) { private Thread recordStream(StringBuffer builder, InputStream inputStream) { return new Thread( () -> { - try (Reader reader = new InputStreamReader(inputStream)) { + try (Reader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8)) { int len; char[] buf = new char[1024]; while ((len = reader.read(buf)) > 0) { if (Runtime.getRuntime().freeMemory() < Runtime.getRuntime().totalMemory() / 2) { - Runtime.getRuntime().gc(); - if (Runtime.getRuntime().freeMemory() < Runtime.getRuntime().totalMemory() / 2) - builder.delete(0, builder.length() / 2); + builder.delete(0, builder.length() / 2); + builder.insert(0, "[earlier messages were removed to free up memory]%n"); } builder.append(buf, 0, len); } diff --git a/core/src/testFixtures/java/org/lflang/tests/TestBase.java b/core/src/testFixtures/java/org/lflang/tests/TestBase.java index 9fb8b93dc0..0c78c8b005 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestBase.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestBase.java @@ -9,7 +9,6 @@ import java.io.BufferedWriter; import java.io.File; import java.io.IOException; -import java.io.PrintStream; import java.io.PrintWriter; import java.io.StringWriter; import java.lang.reflect.Constructor; @@ -67,12 +66,6 @@ public abstract class TestBase extends LfInjectedTestBase { @Inject TestRegistry testRegistry; - /** Reference to System.out. */ - private static final PrintStream out = System.out; - - /** Reference to System.err. */ - private static final PrintStream err = System.err; - /** Execution timeout enforced for all tests. */ private static final long MAX_EXECUTION_TIME_SECONDS = 300; @@ -283,24 +276,6 @@ protected static boolean isLinux() { return OS.contains("linux"); } - /** End output redirection. */ - private static void restoreOutputs() { - System.out.flush(); - System.err.flush(); - System.setOut(out); - System.setErr(err); - } - - /** - * Redirect outputs to the given tests for recording. - * - * @param test The test to redirect outputs to. - */ - private static void redirectOutputs(LFTest test) { - System.setOut(new PrintStream(test.getOutputStream())); - System.setErr(new PrintStream(test.getOutputStream())); - } - /** * Run a test, print results on stderr. * @@ -694,7 +669,7 @@ private void validateAndRun(Set tests, Configurator configurator, TestLe for (var test : tests) { try { - redirectOutputs(test); + test.redirectOutputs(); configure(test, configurator, level); validate(test); if (level.compareTo(TestLevel.CODE_GEN) >= 0) { @@ -710,7 +685,7 @@ private void validateAndRun(Set tests, Configurator configurator, TestLe test.handleTestError( new TestError("Unknown exception during test execution", Result.TEST_EXCEPTION, e)); } finally { - restoreOutputs(); + test.restoreOutputs(); } done++; while (Math.floor(done * x) >= marks && marks < 78) { From f14d6c93f31f27e6e00228dab5ebda6f9804205e Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 14:44:46 +0200 Subject: [PATCH 07/12] clean up LFTest --- .../java/org/lflang/tests/RunSingleTest.java | 2 +- .../java/org/lflang/tests/LFTest.java | 19 ++----------------- .../java/org/lflang/tests/TestRegistry.java | 5 ++--- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/core/src/integrationTest/java/org/lflang/tests/RunSingleTest.java b/core/src/integrationTest/java/org/lflang/tests/RunSingleTest.java index 29e62e835b..fe813823bd 100644 --- a/core/src/integrationTest/java/org/lflang/tests/RunSingleTest.java +++ b/core/src/integrationTest/java/org/lflang/tests/RunSingleTest.java @@ -70,7 +70,7 @@ public void runSingleTest() throws FileNotFoundException { Class testClass = getTestInstance(target); - LFTest testCase = new LFTest(target, path.toAbsolutePath()); + LFTest testCase = new LFTest(path.toAbsolutePath()); TestBase.runSingleTestAndPrintResults(testCase, testClass, TestBase.pathToLevel(path)); } diff --git a/core/src/testFixtures/java/org/lflang/tests/LFTest.java b/core/src/testFixtures/java/org/lflang/tests/LFTest.java index fb2198d8be..e99fee4b54 100644 --- a/core/src/testFixtures/java/org/lflang/tests/LFTest.java +++ b/core/src/testFixtures/java/org/lflang/tests/LFTest.java @@ -6,7 +6,6 @@ import java.nio.file.Paths; import org.eclipse.xtext.util.RuntimeIOException; import org.lflang.FileConfig; -import org.lflang.Target; import org.lflang.generator.LFGeneratorContext; /** @@ -40,29 +39,19 @@ public class LFTest implements Comparable { /** String builder for collecting issues encountered during test execution. */ private final StringBuilder issues = new StringBuilder(); - /** The target of the test program. */ - private final Target target; - private long executionTimeNanoseconds; /** * Create a new test. * - * @param target The target of the test program. * @param srcFile The path to the file of the test program. */ - public LFTest(Target target, Path srcFile) { - this.target = target; + public LFTest(Path srcFile) { this.srcPath = srcFile; this.name = FileConfig.findPackageRoot(srcFile, s -> {}).relativize(srcFile).toString(); this.relativePath = Paths.get(name); } - /** Copy constructor */ - public LFTest(LFTest test) { - this(test.target, test.srcPath); - } - public FileConfig getFileConfig() { return context.getFileConfig(); } @@ -141,11 +130,7 @@ public boolean hasPassed() { return result == Result.TEST_PASS; } - /** - * Compile a string that contains all collected errors and return it. - * - * @return A string that contains all collected errors. - */ + /** Compile a string that contains all collected errors and return it. */ public void reportErrors() { if (this.hasFailed()) { System.out.println( diff --git a/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java b/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java index 17b9984652..e5bb3334cc 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java @@ -116,7 +116,7 @@ public Set getRegisteredTests(Target target, TestCategory category, bool if (copy) { Set copies = new TreeSet<>(); for (LFTest test : registered.getTests(target, category)) { - copies.add(new LFTest(test)); + copies.add(new LFTest(test.getSrcPath())); } return copies; } else { @@ -237,8 +237,7 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes attr) { if (attr.isRegularFile() && path.toString().endsWith(".lf")) { // Try to parse the file. Resource r = rs.getResource(URI.createFileURI(path.toFile().getAbsolutePath()), true); - // FIXME: issue warning if target doesn't match! - LFTest test = new LFTest(target, path); + LFTest test = new LFTest(path); Iterator reactors = IteratorExtensions.filter(r.getAllContents(), Reactor.class); From b09b8b10269f35f5fa2ae8bad826758c453ae257 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 17:22:38 +0200 Subject: [PATCH 08/12] fix warnings in TestRegistry --- .../java/org/lflang/tests/TestRegistry.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java b/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java index e5bb3334cc..80c20baafd 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java @@ -17,10 +17,12 @@ import java.util.EnumMap; import java.util.HashMap; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.Stack; import java.util.TreeSet; + import org.eclipse.emf.common.util.URI; import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.resource.ResourceSet; @@ -42,9 +44,8 @@ public class TestRegistry { * List of directories that should be skipped when indexing test files. Any test file that has a * directory in its path that matches an entry in this array will not be discovered. */ - public static final String[] IGNORED_DIRECTORIES = { - "failing", "knownfailed", "failed", "fed-gen" - }; + public static final List IGNORED_DIRECTORIES = + List.of("failing", "knownfailed", "failed", "fed-gen"); /** Path to the root of the repository. */ public static final Path LF_REPO_PATH = Paths.get("").toAbsolutePath(); @@ -204,7 +205,8 @@ public TestDirVisitor(ResourceSet rs, Target target, Path srcBasePath) { @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { for (String ignored : IGNORED_DIRECTORIES) { - if (dir.getFileName().toString().equalsIgnoreCase(ignored)) { + final var name = dir.getFileName(); + if (name != null && name.toString().equalsIgnoreCase(ignored)) { return SKIP_SUBTREE; } } From 06ee3261e9bdaeeb4d5ea8939d6368cbe99a2d79 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 17:50:54 +0200 Subject: [PATCH 09/12] fix spotbugs and idea warnings in TestBase --- .../java/org/lflang/tests/TestBase.java | 57 +++++++------------ 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/core/src/testFixtures/java/org/lflang/tests/TestBase.java b/core/src/testFixtures/java/org/lflang/tests/TestBase.java index 0c78c8b005..a589beb882 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestBase.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestBase.java @@ -88,8 +88,6 @@ public abstract class TestBase extends LfInjectedTestBase { * @author Marten Lohstroh */ public enum TestLevel { - VALIDATION, - CODE_GEN, BUILD, EXECUTION } @@ -100,10 +98,11 @@ public enum TestLevel { * @author Anirudh Rengarajan */ public static TestLevel pathToLevel(Path path) { - while (path.getParent() != null) { - String name = path.getFileName().toString(); + path = path.getParent(); + while (path != null) { + final var name = path.getFileName(); for (var category : TestCategory.values()) { - if (category.name().equalsIgnoreCase(name)) { + if (name != null && name.toString().equalsIgnoreCase(category.name())) { return category.level; } } @@ -127,13 +126,11 @@ public static class Message { public static final String NO_ENCLAVE_SUPPORT = "Targeet does not support the enclave feature."; public static final String NO_DOCKER_SUPPORT = "Target does not support the 'docker' property."; public static final String NO_DOCKER_TEST_SUPPORT = "Docker tests are only supported on Linux."; - public static final String NO_GENERICS_SUPPORT = "Target does not support generic types."; /* Descriptions of collections of tests. */ public static final String DESC_SERIALIZATION = "Run serialization tests."; public static final String DESC_BASIC = "Run basic tests."; public static final String DESC_GENERICS = "Run generics tests."; - public static final String DESC_TYPE_PARMS = "Run tests for reactors with type parameters."; public static final String DESC_MULTIPORT = "Run multiport tests."; public static final String DESC_AS_FEDERATED = "Run non-federated tests in federated mode."; public static final String DESC_FEDERATED = "Run federated tests."; @@ -151,11 +148,6 @@ public static class Message { public static final String DESC_ROS2 = "Running tests using ROS2."; public static final String DESC_MODAL = "Run modal reactor tests."; public static final String DESC_VERIFIER = "Run verifier tests."; - - /* Missing dependency messages */ - public static final String MISSING_DOCKER = - "Executable 'docker' not found or 'docker' daemon thread not running"; - public static final String MISSING_ARDUINO_CLI = "Executable 'arduino-cli' not found"; } /** Constructor for test classes that test a single target. */ @@ -336,7 +328,7 @@ private static void checkAndReportFailures(Set tests) { var passed = tests.stream().filter(LFTest::hasPassed).toList(); var s = new StringBuffer(); s.append(THIN_LINE); - s.append("Passing: ").append(passed.size()).append("/").append(tests.size()).append("\n"); + s.append("Passing: ").append(passed.size()).append("/").append(tests.size()).append("%n"); s.append(THIN_LINE); passed.forEach( test -> @@ -344,7 +336,7 @@ private static void checkAndReportFailures(Set tests) { .append(test) .append( String.format( - " in %.2f seconds\n", test.getExecutionTimeNanoseconds() / 1.0e9))); + " in %.2f seconds%n", test.getExecutionTimeNanoseconds() / 1.0e9))); s.append(THIN_LINE); System.out.print(s); @@ -357,16 +349,14 @@ private static void checkAndReportFailures(Set tests) { } /** - * Configure a test by applying the given configurator and return a generator context. Also, if - * the given level is less than {@code TestLevel.BUILD}, add a {@code no-compile} flag to the - * generator context. If the configurator was not applied successfully, throw an AssertionError. + * Configure a test by applying the given configurator and return a generator context. + * If the configurator was not applied successfully, throw an AssertionError. * * @param test the test to configure. * @param configurator The configurator to apply to the test. - * @param level The level of testing in which the generator context will be used. */ - private void configure(LFTest test, Configurator configurator, TestLevel level) - throws IOException, TestError { + private void configure(LFTest test, Configurator configurator) + throws TestError { var props = new Properties(); props.setProperty("hierarchical-bin", "true"); @@ -412,11 +402,6 @@ private void configure(LFTest test, Configurator configurator, TestLevel level) test.configure(context); - // Set the no-compile flag the test is not supposed to reach the build stage. - if (level.compareTo(TestLevel.BUILD) < 0) { - context.getArgs().setProperty("no-compile", ""); - } - // Reload in case target properties have changed. context.loadTargetConfig(); // Update the test by applying the configuration. E.g., to carry out an AST transformation. @@ -462,9 +447,9 @@ protected void addExtraLfcArgs(Properties args, TargetConfig targetConfig) { * * @param test The test to generate code for. */ - private GeneratorResult generateCode(LFTest test) throws TestError { + private void generateCode(LFTest test) throws TestError { if (test.getFileConfig().resource == null) { - return GeneratorResult.NOTHING; + test.getContext().finish(GeneratorResult.NOTHING); } try { generator.doGenerate(test.getFileConfig().resource, fileAccess, test.getContext()); @@ -474,8 +459,6 @@ private GeneratorResult generateCode(LFTest test) throws TestError { if (generator.errorsOccurred()) { throw new TestError("Code generation unsuccessful.", Result.CODE_GEN_FAIL); } - - return test.getContext().getResult(); } /** @@ -507,13 +490,13 @@ private void execute(LFTest test) throws TestError { throw new TestError(Result.TEST_TIMEOUT); } else { if (stdoutException.get() != null || stderrException.get() != null) { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); if (stdoutException.get() != null) { - sb.append("Error during stdout handling:" + System.lineSeparator()); + sb.append("Error during stdout handling:%n"); sb.append(stackTraceToString(stdoutException.get())); } if (stderrException.get() != null) { - sb.append("Error during stderr handling:" + System.lineSeparator()); + sb.append("Error during stderr handling:%n"); sb.append(stackTraceToString(stderrException.get())); } throw new TestError(sb.toString(), Result.TEST_EXCEPTION); @@ -551,7 +534,7 @@ public static String stackTraceToString(Throwable t) { } /** Bash script that is used to execute docker tests. */ - private static String DOCKER_RUN_SCRIPT = + private static final String DOCKER_RUN_SCRIPT = """ #!/bin/bash @@ -584,7 +567,7 @@ public static String stackTraceToString(Throwable t) { * *

If the script does not yet exist, it is created. */ - private Path getDockerRunScript() throws TestError { + private static synchronized Path getDockerRunScript() throws TestError { if (dockerRunScript != null) { return dockerRunScript; } @@ -670,11 +653,9 @@ private void validateAndRun(Set tests, Configurator configurator, TestLe for (var test : tests) { try { test.redirectOutputs(); - configure(test, configurator, level); + configure(test, configurator); validate(test); - if (level.compareTo(TestLevel.CODE_GEN) >= 0) { - generateCode(test); - } + generateCode(test); if (level == TestLevel.EXECUTION) { execute(test); } From 89c13d2ae8f28ca56254cd688f0cc0cb2c6f3d67 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 18:00:42 +0200 Subject: [PATCH 10/12] fix spotbugs warnings in issue reporting --- .../src/test/kotlin/org/lflang/cli/LfcIssueReportingTest.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/lfc/src/test/kotlin/org/lflang/cli/LfcIssueReportingTest.kt b/cli/lfc/src/test/kotlin/org/lflang/cli/LfcIssueReportingTest.kt index c2839673be..21cdaf3043 100644 --- a/cli/lfc/src/test/kotlin/org/lflang/cli/LfcIssueReportingTest.kt +++ b/cli/lfc/src/test/kotlin/org/lflang/cli/LfcIssueReportingTest.kt @@ -38,7 +38,9 @@ import java.nio.file.Paths class SpyPrintStream { private val baout = ByteArrayOutputStream() - val ps = PrintStream(baout) + private val ps = PrintStream(baout, false, StandardCharsets.UTF_8) + + fun getSpiedErrIo(): Io = Io(err = ps) override fun toString(): String = baout.toString(StandardCharsets.UTF_8) } @@ -111,7 +113,7 @@ class LfcIssueReportingTest { val stderr = SpyPrintStream() - val io = Io(err = stderr.ps) + val io = stderr.getSpiedErrIo() val backend = ReportingBackend(io, AnsiColors(useColors).bold("lfc: "), AnsiColors(useColors), 2) val injector = LFStandaloneSetup(LFRuntimeModule(), LFStandaloneModule(backend, io)) .createInjectorAndDoEMFRegistration() From eddd59360dafe5a023a445b0c21e137dc07321fc Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Thu, 14 Sep 2023 18:10:05 +0200 Subject: [PATCH 11/12] formatting --- core/src/testFixtures/java/org/lflang/tests/TestBase.java | 7 +++---- .../testFixtures/java/org/lflang/tests/TestRegistry.java | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/testFixtures/java/org/lflang/tests/TestBase.java b/core/src/testFixtures/java/org/lflang/tests/TestBase.java index a589beb882..ee0d72ff02 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestBase.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestBase.java @@ -349,14 +349,13 @@ private static void checkAndReportFailures(Set tests) { } /** - * Configure a test by applying the given configurator and return a generator context. - * If the configurator was not applied successfully, throw an AssertionError. + * Configure a test by applying the given configurator and return a generator context. If the + * configurator was not applied successfully, throw an AssertionError. * * @param test the test to configure. * @param configurator The configurator to apply to the test. */ - private void configure(LFTest test, Configurator configurator) - throws TestError { + private void configure(LFTest test, Configurator configurator) throws TestError { var props = new Properties(); props.setProperty("hierarchical-bin", "true"); diff --git a/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java b/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java index 80c20baafd..14b12647ad 100644 --- a/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java +++ b/core/src/testFixtures/java/org/lflang/tests/TestRegistry.java @@ -22,7 +22,6 @@ import java.util.Set; import java.util.Stack; import java.util.TreeSet; - import org.eclipse.emf.common.util.URI; import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.resource.ResourceSet; From 600c44db5ba7e4718d77a1ddff2c8306564fcd87 Mon Sep 17 00:00:00 2001 From: Christian Menard Date: Fri, 15 Sep 2023 11:19:00 +0200 Subject: [PATCH 12/12] avoid using a dead dependency (jsr305) --- core/build.gradle | 4 ++++ core/src/main/java/org/lflang/Target.java | 2 +- gradle.properties | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/core/build.gradle b/core/build.gradle index 23b412b6a8..142c347e03 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -44,6 +44,10 @@ dependencies { testImplementation "org.opentest4j:opentest4j:$openTest4jVersion" testImplementation "org.eclipse.xtext:org.eclipse.xtext.testing:$xtextVersion" testImplementation "org.eclipse.xtext:org.eclipse.xtext.xbase.testing:$xtextVersion" + + // For spotbugs annotations + compileOnly "com.github.spotbugs:spotbugs-annotations:$spotbugsToolVersion" + compileOnly "net.jcip:jcip-annotations:$jcipVersion" } configurations { diff --git a/core/src/main/java/org/lflang/Target.java b/core/src/main/java/org/lflang/Target.java index c7aadda74f..5e9f72fea1 100644 --- a/core/src/main/java/org/lflang/Target.java +++ b/core/src/main/java/org/lflang/Target.java @@ -24,7 +24,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; -import javax.annotation.concurrent.Immutable; +import net.jcip.annotations.Immutable; import org.lflang.lf.TargetDecl; /** diff --git a/gradle.properties b/gradle.properties index 636741363c..edfd349707 100644 --- a/gradle.properties +++ b/gradle.properties @@ -10,7 +10,6 @@ jacocoVersion=0.8.7 jsonVersion=20200518 jupiterVersion=5.8.2 jUnitPlatformVersion=1.8.2 -klighdVersion=2.3.0.v20230606 kotlinJvmTarget=17 lsp4jVersion=0.21.0 mwe2LaunchVersion=2.14.0 @@ -22,6 +21,7 @@ klighdVersion=2.3.0.v20230606 freehepVersion=2.4 swtVersion=3.124.0 spotbugsToolVersion=4.7.3 +jcipVersion=1.0 [manifestPropertyNames] org.eclipse.xtext=xtextVersion