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

Improvements to EffAssert Logging #6542

Merged
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
23 changes: 18 additions & 5 deletions src/main/java/ch/njol/skript/conditions/CondCompare.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
*/
package ch.njol.skript.conditions;

import ch.njol.skript.lang.VerboseAssert;
import ch.njol.skript.log.ParseLogHandler;
import ch.njol.skript.util.Time;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;

Expand All @@ -29,8 +29,6 @@
import org.skriptlang.skript.lang.comparator.Comparator;
import org.skriptlang.skript.lang.comparator.ComparatorInfo;
import org.skriptlang.skript.lang.comparator.Relation;
import org.skriptlang.skript.lang.converter.ConverterInfo;
import org.skriptlang.skript.lang.converter.Converters;

import ch.njol.skript.doc.Description;
import ch.njol.skript.doc.Examples;
Expand All @@ -45,7 +43,6 @@
import ch.njol.skript.lang.UnparsedLiteral;
import ch.njol.skript.lang.util.SimpleLiteral;
import ch.njol.skript.log.ErrorQuality;
import ch.njol.skript.log.RetainingLogHandler;
import ch.njol.skript.log.SkriptLogger;
import ch.njol.skript.registrations.Classes;

Expand All @@ -66,7 +63,7 @@
"time in the player's world is greater than 8:00",
"the creature is not an enderman or an ender dragon"})
@Since("1.0")
public class CondCompare extends Condition {
public class CondCompare extends Condition implements VerboseAssert {

private final static Patterns<Relation> patterns = new Patterns<>(new Object[][]{
{"(1¦neither|) %objects% ((is|are)(|2¦(n't| not|4¦ neither)) ((greater|more|higher|bigger|larger) than|above)|\\>) %objects%", Relation.GREATER},
Expand Down Expand Up @@ -391,6 +388,22 @@ public boolean check(final Event event) {
), isNegated());
}

public String getExpectedMessage(Event event) {
String message = "a value ";
if (third == null)
return message + (isNegated() ? "not " : "") + relation + " " + VerboseAssert.getExpressionValue(second, event);

// handle between
if (isNegated())
message += "not ";
message += "between " + VerboseAssert.getExpressionValue(second, event) + " and " + VerboseAssert.getExpressionValue(third, event);
return message;
}

public String getReceivedMessage(Event event) {
return VerboseAssert.getExpressionValue(first, event);
}

/**
* Used to directly compare two lists for equality.
* This method assumes that {@link CondCompare#first} and {@link CondCompare#second} are both non-single.
Expand Down
17 changes: 15 additions & 2 deletions src/main/java/ch/njol/skript/conditions/CondIsSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package ch.njol.skript.conditions;

import ch.njol.skript.lang.VerboseAssert;
import ch.njol.skript.localization.Language;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;

Expand All @@ -42,7 +44,7 @@
" projectile exists",
" broadcast \"%attacker% used a %projectile% to attack %victim%!\""})
@Since("1.2")
public class CondIsSet extends Condition {
public class CondIsSet extends Condition implements VerboseAssert {
static {
Skript.registerCondition(CondIsSet.class,
"%~objects% (exist[s]|(is|are) set)",
Expand Down Expand Up @@ -79,7 +81,18 @@ private boolean check(final Expression<?> expr, final Event e) {
public boolean check(final Event e) {
return check(expr, e);
}


@Override
public String getExpectedMessage(Event event) {
return isNegated() ? Language.get("none") : "a value";
}

@Override
public String getReceivedMessage(Event event) {
// TODO: may need to make this enumerate each value: "a, b, <none>, and d"
return VerboseAssert.getExpressionValue(expr,event);
}

@Override
public String toString(final @Nullable Event e, final boolean debug) {
return expr.toString(e, debug) + (isNegated() ? " isn't" : " is") + " set";
Expand Down
60 changes: 60 additions & 0 deletions src/main/java/ch/njol/skript/lang/VerboseAssert.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* This file is part of Skript.
*
* Skript is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Skript is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Skript. If not, see <http://www.gnu.org/licenses/>.
*
* Copyright Peter Güttinger, SkriptLang team and contributors
*/
package ch.njol.skript.lang;

import ch.njol.skript.registrations.Classes;
import org.bukkit.event.Event;

/**
* This interface provides methods for {@link Condition}s to provide expected and received values for {@link ch.njol.skript.test.runner.EffAssert}
* or others to use to in debugging or testing scenarios. <br>
* <br>
* Expected values should be the value being compared against, the source of truth. <br>
* Received values should be the value being tested, the value that may or may not be correct.
*/
public interface VerboseAssert {

/**
* This method is intended to be used directly after {@code "Expected "} and the grammar of the returned string should match.
*
* @param event The event used to evaluate this condition.
* @return The expected value in this condition, formatted as a readable string.
*/
String getExpectedMessage(Event event);

/**
* This method is intended to be used directly after {@code "Expected x, but got "} and the grammar of the returned string should match.
*
* @param event The event used to evaluate this condition.
* @return The received value in this condition, formatted as a readable string.
*/
String getReceivedMessage(Event event);

/**
* Helper method to simplify stringify-ing the values of expressions.
* Evaluates the expression using {@link Expression#getAll(Event)} and stringifies using {@link Classes#toString(Object[], boolean)}.
* @param expression The expression to evaluate
* @param event The event used for evaluation
* @return The string representation of the expression's value.
*/
static String getExpressionValue(Expression<?> expression, Event event) {
return Classes.toString(expression.getAll(event), expression.getAnd());
}

}
54 changes: 49 additions & 5 deletions src/main/java/ch/njol/skript/test/runner/EffAssert.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,14 @@
*/
package ch.njol.skript.test.runner;

import ch.njol.skript.conditions.CondCompare;
import ch.njol.skript.config.Node;
import ch.njol.skript.lang.VerboseAssert;
import ch.njol.skript.registrations.Classes;
import ch.njol.skript.util.LiteralUtils;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;
import org.skriptlang.skript.lang.comparator.Relation;
import org.skriptlang.skript.lang.script.Script;

import ch.njol.skript.Skript;
Expand All @@ -42,28 +48,43 @@ public class EffAssert extends Effect {

static {
if (TestMode.ENABLED)
Skript.registerEffect(EffAssert.class, "assert <.+> [(1¦to fail)] with %string%");
Skript.registerEffect(EffAssert.class,
"assert <.+> [(1:to fail)] with [error] %string%",
"assert <.+> [(1:to fail)] with [error] %string%, expected [value] %object%, [and] (received|got) [value] %object%");
}

@Nullable
private Condition condition;
private Script script;
private int line;

private Expression<String> errorMsg;
@Nullable
private Expression<?> expected;
@Nullable
private Expression<?> got;
private boolean shouldFail;

@Override
@SuppressWarnings("unchecked")
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
String conditionString = parseResult.regexes.get(0).group();
errorMsg = (Expression<String>) exprs[0];
boolean canInit = true;
if (exprs.length > 1) {
expected = LiteralUtils.defendExpression(exprs[1]);
got = LiteralUtils.defendExpression(exprs[2]);
canInit = LiteralUtils.canInitSafely(expected, got);
}
shouldFail = parseResult.mark != 0;
script = getParser().getCurrentScript();
Node node = getParser().getNode();
line = node != null ? node.getLine() : -1;

ParseLogHandler logHandler = SkriptLogger.startParseLogHandler();
try {
condition = Condition.parse(conditionString, "Can't understand this condition: " + conditionString);

if (shouldFail)
return true;

Expand All @@ -75,8 +96,8 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
} finally {
logHandler.stop();
}
return condition != null;

return (condition != null) && canInit;
AyhamAl-Ali marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand All @@ -91,10 +112,33 @@ public TriggerItem walk(Event event) {
if (condition.check(event) == shouldFail) {
String message = errorMsg.getSingle(event);
assert message != null; // Should not happen, developer needs to fix test.

// generate expected/got message if possible
String expectedMessage = "";
String gotMessage = "";
if (expected != null)
expectedMessage = VerboseAssert.getExpressionValue(expected, event);
if (got != null)
gotMessage = VerboseAssert.getExpressionValue(got, event);

if (condition instanceof VerboseAssert) {
if (expectedMessage.isEmpty())
expectedMessage = ((VerboseAssert) condition).getExpectedMessage(event);
if (gotMessage.isEmpty())
gotMessage = ((VerboseAssert) condition).getReceivedMessage(event);
}

if (!expectedMessage.isEmpty() && !gotMessage.isEmpty())
message += " (Expected " + expectedMessage + ", but got " + gotMessage + ")";

if (SkriptJUnitTest.getCurrentJUnitTest() != null) {
TestTracker.junitTestFailed(SkriptJUnitTest.getCurrentJUnitTest(), message);
} else {
TestTracker.testFailed(message, script);
if (line >= 0) {
TestTracker.testFailed(message, script, line);
} else {
TestTracker.testFailed(message, script);
}
}
return null;
}
Expand Down
13 changes: 11 additions & 2 deletions src/main/java/ch/njol/skript/test/runner/TestTracker.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,20 @@ public static void testFailed(String msg) {
}

public static void testFailed(String msg, Script script) {
String file = script.getConfig().getFileName();
file = file.substring(file.lastIndexOf(File.separator) + 1);
String file = getFileName(script);
failedTests.put(currentTest, msg + " [" + file + "]");
}

public static void testFailed(String msg, Script script, int line) {
String file = getFileName(script);
failedTests.put(currentTest, msg + " [" + file + ", line " + line + "]");
}

private static String getFileName(Script script) {
String file = script.getConfig().getFileName();
return file.substring(file.lastIndexOf(File.separator) + 1);
}

public static void junitTestFailed(String junit, String msg) {
failedTests.put(junit, msg);
}
Expand Down
Loading