usethesource / rascal

The implementation of the Rascal meta-programming language (including interpreter, type checker, parser generator, compiler and JVM based run-time system)
http://www.rascal-mpl.org
Other
400 stars 77 forks source link

JunitTestRunner does not report failure (anymore) on static errors during testing #1798

Closed jurgenvinju closed 4 months ago

jurgenvinju commented 1 year ago

Describe the bug

Loading module:lang::cpp::M3
[ERROR] org.rascalmpl.interpreter.staticErrors.UndeclaredJavaMethod: No such Java method: lang.cpp.internal.Parser.parseCppToM3AndAst(io.usethesource.vallang.ISourceLocation, io.usethesource.vallang.IList, io.usethesource.vallang.IList, io.usethesource.vallang.IMap, io.usethesource.vallang.IBool)
Advice: |http://tutor.rascal-mpl.org/Errors/Static/UndeclaredJavaMethod/UndeclaredJavaMethod.html|
Loading module:lang::cpp::TypeSymbol
    skipping. Module has no tests.
Loading module:lang::cpp::ASTgen
    skipping. Module has no tests.
Loading module:lang::cpp::AST
    skipping. Module has no tests.
Loading module:lang::cpp::IDE
    skipping. Module has no tests.
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.088 s - in lang.cpp.internal.RunTests
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

So BUILD SUCCESS is a problem, so is Tests run:0 and so is Failures: 0

jurgenvinju commented 1 year ago

In RascalJunitTestRunner this code looks suspicious:

@Override
    public void run(final RunNotifier notifier) {
        if (desc == null) {
            desc = getDescription();
        }
        notifier.fireTestRunStarted(desc);

        for (Description mod : desc.getChildren()) {
            if (mod.getAnnotations().stream().anyMatch(t -> t instanceof CompilationFailed)) {
                notifier.fireTestFailure(new Failure(desc, new IllegalArgumentException(mod.getDisplayName() + " had importing errors")));
                continue;
            }

            Listener listener = new Listener(notifier, mod);
            TestEvaluator runner = new TestEvaluator(evaluator, listener);
            runner.test(mod.getDisplayName());
        }

        notifier.fireTestRunFinished(new Result());
    }

In particular we are not supposed to call notifier.fireTestRunFinished or notifier.fireTestRunStarted per the JUNit Javadoc on those methods. This is not necessarily the cause of this bug, but it might be.

jurgenvinju commented 1 year ago

Here is the bug in action: https://github.com/usethesource/clair/actions/runs/5132131642/jobs/9233036834