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

Bugfixes in Command-line SPT transform expectation evaluation. #132

Merged
merged 8 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@ All notable changes to this project are documented in this file, based on [Keep


## [Unreleased]
### Changed
- Command Line Interface now returns a non-zero error code when the underlying command fails.
- Command Line Interface executing tests now prints the number of passed and failed test cases.
- SPT `transform` expectations support commands using the enclosing file and/or enclosing project.

### Fixed
- Termination issues in Code Completion
- Serialization issues of meta-language configuration objects
- Region arguments to SPT `transform` expectations now correspond to the imploder attachments.

## [0.19.7] - 2024-02-09
### Changed
Expand Down
19 changes: 13 additions & 6 deletions core/spoofax.cli/src/main/java/mb/spoofax/cli/CommandRunner.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package mb.spoofax.cli;

import mb.common.message.KeyedMessages;
import mb.pie.api.ExecException;
import mb.pie.api.MixedSession;
import mb.pie.api.Task;
import mb.resource.ReadableResource;
Expand Down Expand Up @@ -53,7 +54,7 @@ void set(String paramId, @Nullable Object value) throws IllegalArgumentException
rawArgsBuilder.setArg(paramId, (Serializable)value);
}

@Override public @Nullable Object call() throws Exception {
@Override public @Nullable Object call() throws SpoofaxCliException, InterruptedException, ExecException {
final RawArgs rawArgs = rawArgsBuilder.build(context);
final A args = commandDef.fromRawArgs(rawArgs);
final Task<CommandFeedback> task = commandDef.createTask(args);
Expand All @@ -67,11 +68,12 @@ void set(String paramId, @Nullable Object value) throws IllegalArgumentException

final KeyedMessages keyedMessages = feedback.getMessages();
if(!keyedMessages.isEmpty()) {
System.out.println("The following messages were produced by command '" + commandDef.getDisplayName() + "':\n" + keyedMessages.toString());
System.out.println("The following messages were produced by command '" + commandDef.getDisplayName() + "':\n" + keyedMessages);
}

boolean commandFailed = exception != null || keyedMessages.containsErrorOrHigher();
for(ShowFeedback showFeedback : feedback.getShowFeedbacks()) {
showFeedback.caseOf()
commandFailed |= showFeedback.caseOf()
.showFile((file, region) -> {
try {
final ReadableResource resource = resourceService.getReadableResource(file);
Expand All @@ -85,23 +87,28 @@ void set(String paramId, @Nullable Object value) throws IllegalArgumentException
System.err.println("An exception occurred while showing file '" + file + "':");
e.printStackTrace(System.err);
}
return Optional.empty();
return false;
})
.showText((text, name, region) -> {
if(printFeedbackNames && !name.isEmpty()) {
System.out.println(name + ":");
System.out.println();
}
System.out.println(text);
return Optional.empty();
return false;
})
.showTestResults(((testResults, region) -> {
StringBuilder builder = new StringBuilder();
testResults.addToStringBuilder(builder);
System.out.print(builder);
return Optional.empty();
return testResults.numFailed > 0;
}));
}

if(commandFailed) {
// Exception is processed and turned into an exit code by picocli.
throw new SpoofaxCliException("Command '" + commandDef.getDisplayName() + "' failed (see messages above).", exception);
}
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package mb.spoofax.cli;

import org.checkerframework.checker.nullness.qual.Nullable;

public class SpoofaxCliException extends Exception {

public SpoofaxCliException(String message) {
super(message);
}

public SpoofaxCliException(String message, @Nullable Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public String toJavaCode() {
}


public static class Option {
public static class Option implements Serializable {
public final ListView<String> names;
public final boolean negatable;
public final @Nullable String label;
Expand Down Expand Up @@ -187,7 +187,7 @@ public Option(ListView<String> names, boolean negatable, @Nullable String label,
}
}

public static class Positional {
public static class Positional implements Serializable {
public final int index;
public final @Nullable String label;
public final @Nullable String description;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

import org.immutables.value.Value;

import java.io.Serializable;
import java.util.Optional;

@Value.Immutable
public interface SeparatorRepr {
public interface SeparatorRepr extends Serializable {
class Builder extends ImmutableSeparatorRepr.Builder {}

static Builder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ public void addToStringBuilder(StringBuilder builder) {
for (TestSuiteResult suite : suites) {
suite.addToStringBuilder(builder);
}
builder
.append(numPassed)
.append(" test cases passed, ")
.append(numFailed)
.append(" test cases failed.");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,12 @@ public static Seq<CCSolverState> eval(
if(!unifier.isGround(query.scopeTerm())) {
// Delay
final Delay delay = Delay.ofVars(unifier.getVars(query.scopeTerm()));
return Seq.of(input.withoutSelected().withDelay(query, delay));
return Seq.of(input
.withoutSelected()
// remove query from active constraint set, as it is delayed now
.withUpdatedConstraints(Collections.emptySet(), Collections.singleton(query))
.withDelay(query, delay)
);
}
@Nullable final Scope scope = Scope.matcher().match(query.scopeTerm(), unifier).orElse(null);
assert scope != null;
Expand Down Expand Up @@ -353,7 +358,12 @@ public F1<Scope, Env<Scope, ITerm, ITerm>> caseCompiledQuery(CCompiledQuery q) {
} catch(IncompleteException e) {
// Delay
final Delay delay = Delay.ofVars(unifier.getVars(query.scopeTerm()));
return Collections.singletonList(input.withoutSelected().withDelay(query, delay));
return Collections.singletonList(input
.withoutSelected()
// remove query from active constraint set, as it is delayed now
.withUpdatedConstraints(Collections.emptySet(), Collections.singleton(query))
.withDelay(query, delay)
);
} catch(ResolutionException e) {
throw new RuntimeException("Unexpected ResolutionException: " + e.getMessage(), e);
} catch(InterruptedException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spt.lut.LanguageUnderTestProvider;
import mb.spt.model.LanguageUnderTest;
import mb.spt.model.SelectionReference;
import mb.spt.model.TestCase;
import mb.spt.model.TestExpectation;
import mb.spt.util.SptMessageRemap;
import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.ArrayList;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -77,6 +79,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spt.lut.LanguageUnderTestProvider;
import mb.spt.model.LanguageUnderTest;
import mb.spt.model.SelectionReference;
import mb.spt.model.TestCase;
import mb.spt.model.TestExpectation;
import mb.spt.util.SptMessageRemap;
import org.checkerframework.checker.nullness.qual.Nullable;

import java.util.ArrayList;
import java.util.stream.Collectors;
Expand All @@ -39,6 +41,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spoofax.core.language.LanguageInstance;
import mb.spt.api.parse.ParseResult;
import mb.spt.api.parse.TestableParse;
Expand All @@ -17,6 +18,7 @@
import mb.spt.model.TestCase;
import mb.spt.model.TestExpectation;
import mb.spt.util.SptMessageRemap;
import org.checkerframework.checker.nullness.qual.Nullable;

public class ParseExpectation implements TestExpectation {
public enum Recovery {
Expand Down Expand Up @@ -49,6 +51,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,19 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spoofax.core.language.LanguageInstance;
import mb.spt.lut.LanguageUnderTestProvider;
import mb.spt.model.LanguageUnderTest;
import mb.spt.model.TestCase;
import mb.spt.api.parse.TestableParse;
import mb.spt.model.TestExpectation;
import mb.spt.util.SptAtermMatcher;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.spoofax.interpreter.terms.IStrategoTerm;
import org.spoofax.terms.TermFactory;


public class ParseToAtermExpectation implements TestExpectation {
private final IStrategoTerm expectedMatch;
private final Region sourceRegion;
Expand All @@ -35,6 +38,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spoofax.core.CoordinateRequirement;
import mb.spoofax.core.language.LanguageInstance;
import mb.spt.api.parse.TestableParse;
Expand Down Expand Up @@ -38,6 +39,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spoofax.core.language.LanguageInstance;
import mb.spt.api.resolve.TestableResolve;
import mb.spt.lut.LanguageUnderTestProvider;
Expand All @@ -18,6 +19,7 @@
import mb.spt.model.TestCase;
import mb.spt.model.TestExpectation;
import mb.spt.util.SptSelectionUtil;
import org.checkerframework.checker.nullness.qual.Nullable;

public class ResolveExpectation implements TestExpectation {
public final SelectionReference fromTerm;
Expand All @@ -44,6 +46,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spoofax.core.language.LanguageInstance;
import mb.spt.api.analyze.StrategoRunArgument;
import mb.spt.api.stratego.TestableStratego;
Expand All @@ -21,6 +22,7 @@
import mb.spt.model.TestCase;
import mb.spt.model.TestExpectation;
import mb.stratego.common.StrategoException;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.spoofax.interpreter.terms.IStrategoAppl;
import org.spoofax.interpreter.terms.IStrategoInt;
import org.spoofax.interpreter.terms.IStrategoString;
Expand Down Expand Up @@ -62,6 +64,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import mb.pie.api.Session;
import mb.pie.api.exec.CancelToken;
import mb.resource.ResourceKey;
import mb.resource.hierarchical.ResourcePath;
import mb.spoofax.core.language.command.CommandDef;
import mb.spoofax.core.language.command.CommandFeedback;
import mb.spt.lut.LanguageUnderTestProvider;
Expand Down Expand Up @@ -37,6 +38,7 @@ public KeyedMessages evaluate(
LanguageUnderTest languageUnderTest,
Session languageUnderTestSession,
LanguageUnderTestProvider languageUnderTestProvider,
@Nullable ResourcePath rootDirectoryHint,
ExecContext context,
CancelToken cancel
) throws InterruptedException {
Expand All @@ -55,7 +57,7 @@ public KeyedMessages evaluate(
final @Nullable Region selectionRegion = TransformExpectationUtil.getSelection(testCase, selectionReference);

final @Nullable CommandFeedback feedback = TransformExpectationUtil.runCommand(testCase.resource, commandDef,
languageUnderTest, languageUnderTestSession, messagesBuilder, file, sourceRegion, selectionRegion);
languageUnderTest, languageUnderTestSession, messagesBuilder, file, rootDirectoryHint, sourceRegion, selectionRegion);
if(feedback == null) {
return messagesBuilder.build(file);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,17 @@ public class TransformExpectationUtil {
Session languageUnderTestSession,
KeyedMessagesBuilder messagesBuilder,
ResourceKey failMessageFile,
@Nullable ResourcePath rootDirectoryHint,
Region fileMessageRegion,
@Nullable Region selection
) throws InterruptedException {
try {
final CommandContext commandContext = CommandContext.ofReadableResource(resource, selection);
// Using `ofFile` is required to make `transform` test expectations work.
// Otherwise, the argument provider `Context(File)` will not pick up this resource.
final CommandContext commandContext = CommandContext.ofFile(resource, selection);
commandContext.setEnclosing(EnclosingCommandContextType.Directory, CommandContext.ofDirectory(resource));
commandContext.setEnclosing(EnclosingCommandContextType.Project, CommandContext.ofProject(resource));
// If no root directory hint is given, use the resource itself as the project context.
commandContext.setEnclosing(EnclosingCommandContextType.Project, CommandContext.ofProject(rootDirectoryHint != null ? rootDirectoryHint : resource));
final Task<CommandFeedback> task = commandDef.createTask(CommandExecutionType.ManualOnce, commandContext, new ArgConverters(languageUnderTest.getResourceServiceComponent().getResourceService()));
return languageUnderTestSession.require(task);
} catch(ExecException | ArgumentBuilderException e) {
Expand All @@ -82,7 +86,9 @@ public static boolean isSelectionValid(TestCase testCase, Option<SelectionRefere
if(selectionReference.isNone()) {
return null;
} else {
final ListView<Region> availableSelections = testCase.testFragment.getInFragmentSelections();
// `getSelections()` returns the mapped regions, as the object language parser sees them.
// This corresponds to the regions Stratego strategies such as `origin-offset` etc. return.
final ListView<Region> availableSelections = testCase.testFragment.getSelections();
return availableSelections.get(selectionReference.get().selection - 1);
}
}
Expand Down
Loading