Skip to content

Commit

Permalink
Support test names by comment (#148)
Browse files Browse the repository at this point in the history 
Partially fixes #141.
  • Loading branch information
@netdpb
netdpb authored Feb 6, 2024
1 parent 388343b commit 7755009
Show file tree
Hide file tree
Showing 6 changed files with 575 additions and 390 deletions.
3 changes: 2 additions & 1 deletion conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestReport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private boolean isUnexpected(ReportedFact reportedFact) {

private static void writeFact(Formatter report, Fact fact, String status) {
report.format(
"%s: %s:%d %s%n", status, fact.getFile(), fact.getLineNumber(), fact.getFactText());
"%s: %s:%s:%s%n", status, fact.getFile(), fact.getIdentifier(), fact.getFactText());
}

/** A builder for {@link ConformanceTestReport}s. */
Expand Down Expand Up @@ -191,6 +191,7 @@ private Stream<ImmutableMap.Entry<ExpectedFact, ReportedFact>> matchFacts(

/** Builds the report. */
ConformanceTestReport build() {
expectedFactReader.checkErrors();
return new ConformanceTestReport(
files.build(),
index(expectedFacts.build(), Fact::getFile),
Expand Down
95 changes: 86 additions & 9 deletions conformance-test-framework/src/main/java/org/jspecify/conformance/ExpectedFact.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,23 @@
package org.jspecify.conformance;

import static com.google.common.base.MoreObjects.toStringHelper;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
import static java.util.stream.Collectors.joining;

import com.google.common.base.CharMatcher;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.jspecify.annotations.Nullable;

/**
* An assertion that the tool behaves in a way consistent with a specific fact about a line in the
Expand All @@ -47,11 +53,14 @@ public final class ExpectedFact extends Fact {
Pattern.compile("test:irrelevant-annotation:\\S+"),
Pattern.compile("test:sink-type:[^:]+:.*"));

private final @Nullable String testName;
private final String factText;
private final long factLineNumber;

ExpectedFact(Path file, long lineNumber, String factText, long factLineNumber) {
ExpectedFact(
Path file, long lineNumber, @Nullable String testName, String factText, long factLineNumber) {
super(file, lineNumber);
this.testName = testName;
this.factText = factText;
this.factLineNumber = factLineNumber;
}
Expand All @@ -73,6 +82,11 @@ long getFactLineNumber() {
return factLineNumber;
}

@Override
public String getIdentifier() {
return testName == null ? super.getIdentifier() : testName;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -84,19 +98,21 @@ public boolean equals(Object o) {
ExpectedFact that = (ExpectedFact) o;
return this.getFile().equals(that.getFile())
&& this.getLineNumber() == that.getLineNumber()
&& Objects.equals(this.testName, that.testName)
&& this.factText.equals(that.factText)
&& this.factLineNumber == that.factLineNumber;
}

@Override
public int hashCode() {
return Objects.hash(getFile(), getLineNumber(), factText);
return Objects.hash(getFile(), getLineNumber(), testName, factText);
}

@Override
public String toString() {
return toStringHelper(this)
.add("file", getFile())
.add("testName", testName)
.add("lineNumber", getLineNumber())
.add("factText", factText)
.add("factLineNumber", factLineNumber)
Expand All @@ -108,35 +124,96 @@ static class Reader {

private static final Pattern EXPECTATION_COMMENT =
Pattern.compile(
"\\s*// "
"\\s*// ("
+ "(test:name:(?<testName>.*))"
+ "|"
+ ("(?<fact>"
+ FACT_PATTERNS.stream().map(Pattern::pattern).collect(joining("|"))
+ ")")
+ "\\s*");
+ ")\\s*");

private static final CharMatcher ASCII_DIGIT = CharMatcher.inRange('0', '9');

private final Map<Long, String> facts = new HashMap<>();
private final List<String> errors = new ArrayList<>();

private Path file;
private @Nullable String testName;
private long lineNumber;

/** Reads expected facts from lines in a file. */
ImmutableList<ExpectedFact> readExpectedFacts(Path file, List<String> lines) {
this.file = file;
ImmutableList.Builder<ExpectedFact> expectedFacts = ImmutableList.builder();
Map<Long, String> facts = new HashMap<>();
ListIterator<String> i = lines.listIterator();
while (i.hasNext()) {
String line = i.next();
long lineNumber = i.nextIndex();
lineNumber = i.nextIndex();
Matcher matcher = EXPECTATION_COMMENT.matcher(line);
if (matcher.matches()) {
setTestName(matcher.group("testName"));
String fact = matcher.group("fact");
if (fact != null) {
facts.put(lineNumber, fact.trim());
}
} else {
if (testName != null) {
check(!facts.isEmpty(), "no expected facts for test named %s", testName);
}
facts.forEach(
(factLineNumber, factText) ->
expectedFacts.add(new ExpectedFact(file, lineNumber, factText, factLineNumber)));
expectedFacts.add(
new ExpectedFact(file, lineNumber, testName, factText, factLineNumber)));
facts.clear();
testName = null;
}
}
// TODO(netdpb): Report an error if facts is not empty.
return expectedFacts.build();
return checkUniqueTestNames(expectedFacts.build());
}

private void setTestName(@Nullable String testName) {
if (testName == null) {
return;
}
check(this.testName == null, "test name already set");
check(facts.isEmpty(), "test name must come before assertions for a line");
this.testName = checkTestName(testName.trim());
}

private boolean check(boolean test, String format, Object... args) {
if (!test) {
errors.add(String.format(" %s:%d: %s", file, lineNumber, String.format(format, args)));
}
return test;
}

private String checkTestName(String testName) {
if (check(!testName.isEmpty(), "test name cannot be empty")) {
check(!testName.contains(":"), "test name cannot contain a colon: %s", testName);
check(!ASCII_DIGIT.matchesAllOf(testName), "test name cannot be an integer: %s", testName);
}
return testName;
}

private ImmutableList<ExpectedFact> checkUniqueTestNames(
ImmutableList<ExpectedFact> expectedFacts) {
expectedFacts.stream()
.filter(ef -> ef.testName != null)
.collect(toImmutableSetMultimap(ef -> ef.testName, ExpectedFact::getLineNumber))
.asMap()
.forEach(
(testName, lineNumbers) ->
check(
lineNumbers.size() == 1,
"test name not unique: test '%s' appears on tests of lines %s",
testName,
lineNumbers));
return expectedFacts;
}

/** Throws if there were any errors encountered while reading expected facts. */
void checkErrors() {
checkArgument(errors.isEmpty(), "errors in test inputs:\n%s", Joiner.on('\n').join(errors));
}
}
}
5 changes: 5 additions & 0 deletions conformance-test-framework/src/main/java/org/jspecify/conformance/Fact.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,9 @@ final long getLineNumber() {

/** The text form of the fact. */
protected abstract String getFactText();

/** Returns an object that helps to identify this fact within a file. */
public String getIdentifier() {
return String.valueOf(getLineNumber());
}
}
114 changes: 108 additions & 6 deletions conformance-test-framework/src/test/java/org/jspecify/conformance/ExpectedFactTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static com.google.common.truth.Truth.assertThat;
import static java.util.Arrays.asList;
import static org.junit.Assert.assertThrows;

import com.google.common.collect.ImmutableList;
import java.nio.file.Path;
Expand Down Expand Up @@ -50,12 +51,113 @@ public void readExpectedFacts() {
"// test:irrelevant-annotation:@Nullable ",
"yet another line under test"))
.containsExactly(
new ExpectedFact(FILE, 3, "jspecify_nullness_mismatch", 1),
new ExpectedFact(FILE, 3, "jspecify_nullness_mismatch plus_other_stuff", 2),
new ExpectedFact(FILE, 7, "test:cannot-convert:type1 to type2", 4),
new ExpectedFact(FILE, 7, "test:expression-type:type2:expr2", 5),
new ExpectedFact(FILE, 7, "test:sink-type:type1:expr1", 6),
new ExpectedFact(FILE, 9, "test:irrelevant-annotation:@Nullable", 8))
new ExpectedFact(FILE, 3, null, "jspecify_nullness_mismatch", 1),
new ExpectedFact(FILE, 3, null, "jspecify_nullness_mismatch plus_other_stuff", 2),
new ExpectedFact(FILE, 7, null, "test:cannot-convert:type1 to type2", 4),
new ExpectedFact(FILE, 7, null, "test:expression-type:type2:expr2", 5),
new ExpectedFact(FILE, 7, null, "test:sink-type:type1:expr1", 6),
new ExpectedFact(FILE, 9, null, "test:irrelevant-annotation:@Nullable", 8))
.inOrder();
reader.checkErrors();
}

@Test
public void readExpectedFacts_name() {
assertThat(
readExpectedFacts(
"// test:name: test 1 ",
"// jspecify_nullness_mismatch ",
"// jspecify_nullness_mismatch plus_other_stuff ",
"line under test",
"// test:name: test 2 ",
"// test:cannot-convert:type1 to type2 ",
"// test:expression-type:type2:expr2 ",
"// test:sink-type:type1:expr1 ",
"another line under test",
"// test:name: test 3 ",
"// test:irrelevant-annotation:@Nullable ",
"yet another line under test"))
.containsExactly(
new ExpectedFact(FILE, 4, "test 1", "jspecify_nullness_mismatch", 2),
new ExpectedFact(FILE, 4, "test 1", "jspecify_nullness_mismatch plus_other_stuff", 3),
new ExpectedFact(FILE, 9, "test 2", "test:cannot-convert:type1 to type2", 6),
new ExpectedFact(FILE, 9, "test 2", "test:expression-type:type2:expr2", 7),
new ExpectedFact(FILE, 9, "test 2", "test:sink-type:type1:expr1", 8),
new ExpectedFact(FILE, 12, "test 3", "test:irrelevant-annotation:@Nullable", 11))
.inOrder();
reader.checkErrors();
}

@Test
public void readExpectedFacts_name_empty_throws() {
ImmutableList<ExpectedFact> unused = readExpectedFacts("// test:name: ");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("test name cannot be empty");
}

@Test
public void readExpectedFacts_name_allDigits_throws() {
ImmutableList<ExpectedFact> unused = readExpectedFacts("// test:name: 1234 ");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("test name cannot be an integer");
}

@Test
public void readExpectedFacts_name_colon_throws() {
ImmutableList<ExpectedFact> unused = readExpectedFacts("// test:name:has a : colon");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("test name cannot contain a colon");
}

@Test
public void readExpectedFacts_name_notFirst_throws() {
ImmutableList<ExpectedFact> unused =
readExpectedFacts(
"// test:expression-type:type:expr", //
"// test:name:testName",
"line under test");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("test name must come before assertions for a line");
}

@Test
public void readExpectedFacts_name_noFact_throws() {
ImmutableList<ExpectedFact> unused =
readExpectedFacts(
"// test:name:testName", //
"line under test");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("no expected facts");
}

@Test
public void readExpectedFacts_name_second_name() {
ImmutableList<ExpectedFact> unused =
readExpectedFacts(
"// test:name:testName1", //
"// test:name:testName2");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("test name already set");
}

@Test
public void readExpectedFacts_name_notUnique() {
ImmutableList<ExpectedFact> unused =
readExpectedFacts(
"// test:name:testName",
" // test:expression-type:type1:expr1",
"line 1 under test",
"// test:name:testName",
" // test:expression-type:type2:expr2",
"line 2 under test");
assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
.hasMessageThat()
.contains("test name not unique: test 'testName' appears on tests of lines [3, 6]");
}
}
32 changes: 16 additions & 16 deletions tests/ConformanceTest-report.txt
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
# 12 pass; 7 fail; 19 total; 63.2% score
PASS: Basic.java:28 test:expression-type:Object?:nullable
PASS: Basic.java:28 test:sink-type:Object!:return
PASS: Basic.java:28 test:cannot-convert:Object? to Object!
PASS: Basic.java:34 test:expression-type:Object!:nonNull
PASS: Basic.java:34 test:sink-type:Object?:return
PASS: Basic.java:41 test:sink-type:Object?:nullableObject
PASS: Basic.java:43 test:sink-type:String!:testSinkType#nonNullString
FAIL: Basic.java:49 test:expression-type:List!<capture of ? extends String?>:nullableStrings
PASS: Basic.java:28:test:expression-type:Object?:nullable
PASS: Basic.java:28:test:sink-type:Object!:return
PASS: Basic.java:28:test:cannot-convert:Object? to Object!
PASS: Basic.java:34:test:expression-type:Object!:nonNull
PASS: Basic.java:34:test:sink-type:Object?:return
PASS: Basic.java:41:test:sink-type:Object?:nullableObject
PASS: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString
FAIL: Basic.java:49:test:expression-type:List!<capture of ? extends String?>:nullableStrings
PASS: Basic.java: no unexpected facts
PASS: Irrelevant.java:28 test:irrelevant-annotation:Nullable
PASS: Irrelevant.java:34 test:irrelevant-annotation:Nullable
FAIL: Irrelevant.java:38 test:irrelevant-annotation:NonNull
FAIL: Irrelevant.java:43 test:irrelevant-annotation:NonNull
FAIL: Irrelevant.java:43 test:irrelevant-annotation:Nullable
FAIL: Irrelevant.java:47 test:irrelevant-annotation:NullMarked
FAIL: Irrelevant.java:49 test:irrelevant-annotation:NullUnmarked
PASS: Irrelevant.java:28:test:irrelevant-annotation:Nullable
PASS: Irrelevant.java:34:test:irrelevant-annotation:Nullable
FAIL: Irrelevant.java:38:test:irrelevant-annotation:NonNull
FAIL: Irrelevant.java:43:test:irrelevant-annotation:NonNull
FAIL: Irrelevant.java:43:test:irrelevant-annotation:Nullable
FAIL: Irrelevant.java:47:test:irrelevant-annotation:NullMarked
FAIL: Irrelevant.java:49:test:irrelevant-annotation:NullUnmarked
FAIL: Irrelevant.java: no unexpected facts
PASS: UsesDep.java:24 test:cannot-convert:null? to Dep*
PASS: UsesDep.java:24:test:cannot-convert:null? to Dep*
PASS: UsesDep.java: no unexpected facts
Loading

0 comments on commit 7755009

Please sign in to comment.