Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleaned up test code to fix all test related spotbugs warnings #2006

Merged
merged 13 commits into from
Sep 20, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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() {
Expand Down Expand Up @@ -117,9 +122,9 @@ public void verify(ThrowingConsumer<ExecutionResult> 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) {
Expand Down
5 changes: 4 additions & 1 deletion cli/base/src/testFixtures/java/org/lflang/cli/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 4 additions & 0 deletions core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void runSingleTest() throws FileNotFoundException {

Class<? extends TestBase> testClass = getTestInstance(target);

LFTest testCase = new LFTest(target, path.toAbsolutePath());
LFTest testCase = new LFTest(path.toAbsolutePath());

TestBase.runSingleTestAndPrintResults(testCase, testClass, TestBase.pathToLevel(path));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -233,7 +230,13 @@ private void alter(BiFunction<ListIterator<String>, 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);
}
}
}

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
61 changes: 11 additions & 50 deletions core/src/main/java/org/lflang/Target.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,15 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
import net.jcip.annotations.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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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<String> keywords;
private final Set<String> keywords;

/** An unmodifiable list of all known targets. */
public static final List<Target> ALL = List.of(Target.values());
Expand All @@ -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<String> keywords) {
Target(String displayName, boolean requiresTypes, Collection<String> 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<String> keywords) {
this(displayName, requiresTypes, "N/A", "N/A", keywords); // FIXME: prefix
}

/**
Expand Down Expand Up @@ -469,16 +441,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;
};
}

/**
Expand All @@ -494,15 +460,10 @@ 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
*/
/** 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;
Expand Down
Loading
Loading