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

Introduce Rule for tests in runtime-integration-tests #11683

Open
Akirathan opened this issue Nov 27, 2024 · 2 comments
Open

Introduce Rule for tests in runtime-integration-tests #11683

Akirathan opened this issue Nov 27, 2024 · 2 comments
Assignees
Labels
-compiler p-medium Should be completed in the next few sprints

Comments

@Akirathan
Copy link
Member

Akirathan commented Nov 27, 2024

Every test in runtime-integration-tests now has to manually initialize the polyglot context and close it, like in WarningsTest

One has to remember that it is vital that the context is closed and assigned to null so that no memory is leaked when running all the tests on the CI. This issue has been observed in the past and partially fixed in #10793.

If every test that uses polyglot context (which is true for majority of tests in runtime-integration-tests) extends some test base class, it would have the following benefits:

  • Ensuring that the polyglot context is disposed, and GC'ed. This is now done manually, for example in VectorTest.
  • Discarding all logs and output from the compiler and the interpreter in successful tests. See the usage of TestWatcher in DebuggingEnsoTest
    • Warnings, and other logs from the compiler usually clutter output on the CI.
  • Less boilerplate code.

Probably better than a base class would be adding a TestRule and using it in tests with the Rule annotation.

Mentioned in #11468 (comment)

Example custom rule in our code base:

public static final class PrintCodeRule implements TestRule {
@Override
public Statement apply(Statement base, Description description) {
return new Statement() {
@Override
public void evaluate() {
try {
base.evaluate();
} catch (Throwable e) {
var sb = new StringBuilder();
sb.append("Test failed executing code:").append(System.lineSeparator());
sb.append(code).append(System.lineSeparator());
throw new AssertionError(sb.toString(), e);
}
}
};
}
}

@Akirathan Akirathan changed the title Introduce abstract base class for tests in runtime-integration-tests Introduce abstract base class (or rule) for tests in runtime-integration-tests Nov 28, 2024
@JaroslavTulach JaroslavTulach changed the title Introduce abstract base class (or rule) for tests in runtime-integration-tests Introduce Rule for tests in runtime-integration-tests Dec 2, 2024
@JaroslavTulach JaroslavTulach added the p-medium Should be completed in the next few sprints label Dec 3, 2024
@Akirathan
Copy link
Member Author

Another motivation to introduce a testing rule is an Invalid sharing layer AssertionError encountered in DebuggingEnsoTest in #11687. The fix for that was to initialize and dispose the context statically - 193db45. I tried to wrap some testing methods in ContextUtils.executeInContext, but was not successful with that.

@JaroslavTulach
Copy link
Member

diff --git build.sbt build.sbt
index e588843a70..d5b4a24755 100644
--- build.sbt
+++ build.sbt
@@ -3880,6 +3880,7 @@ lazy val `test-utils` =
       annotationProcSetting,
       libraryDependencies ++= GraalVM.modules,
       libraryDependencies ++= Seq(
+        "junit"          % "junit"           % junitVersion   % "provided",
         "org.graalvm.truffle" % "truffle-api"           % graalMavenPackagesVersion % "provided",
         "org.graalvm.truffle" % "truffle-dsl-processor" % graalMavenPackagesVersion % "provided"
       ),
diff --git engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecCompilerTest.java engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecCompilerTest.java
index 4b41e8dfaa..7fadf2629b 100644
--- engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecCompilerTest.java
+++ engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/ExecCompilerTest.java
@@ -17,24 +17,26 @@ import org.enso.common.MethodNames;
 import org.enso.common.MethodNames.Module;
 import org.enso.common.RuntimeOptions;
 import org.enso.compiler.core.ir.expression.errors.Conversion.DeclaredAsPrivate$;
+import org.enso.test.utils.ContextUtils;
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.PolyglotException;
 import org.graalvm.polyglot.Source;
 import org.graalvm.polyglot.io.IOAccess;
 import org.hamcrest.core.AllOf;
 import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.ClassRule;
 import org.junit.Ignore;
 import org.junit.Test;
 
 public class ExecCompilerTest {
-  private static Context ctx;
   private static ByteArrayOutputStream out;
 
-  @BeforeClass
-  public static void initEnsoContext() {
+  @ClassRule
+  public static final ContextUtils ctx = new ContextUtils(ExecCompilerTest::initEnsoContext);
+
+  private static Context initEnsoContext() {
     out = new ByteArrayOutputStream();
-    ctx =
+    var ctx =
         Context.newBuilder()
             .allowExperimentalOptions(true)
             .allowIO(IOAccess.ALL)
@@ -50,20 +52,21 @@ public class ExecCompilerTest {
             .build();
     assertNotNull(
         "Enso language is supported", ctx.getEngine().getLanguages().get(LanguageInfo.ID));
+    return ctx;
   }
 
   @AfterClass
-  public static void closeEnsoContext() throws Exception {
-    ctx.close();
+  public static void resetOutput() throws Exception {
     out.reset();
   }
 
   @Test
   public void testCaseOfWithNegativeConstant() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     run value =
         case value of
             -1 -> "minus one"
@@ -78,7 +81,7 @@ public class ExecCompilerTest {
 
   @Test
   public void testDesugarOperators() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module = ctx.ctx().eval(LanguageInfo.ID, """
     main =
       ma ==ums==
     """);
@@ -92,7 +95,7 @@ public class ExecCompilerTest {
 
   @Test
   public void testDesugarOperatorsLeftRight() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module = ctx.ctx().eval(LanguageInfo.ID, """
     main = (+ (2 *))
     """);
     try {
@@ -105,7 +108,7 @@ public class ExecCompilerTest {
 
   @Test
   public void testDesugarOperatorsRightLeft() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module = ctx.ctx().eval(LanguageInfo.ID, """
     main = ((* 2) +)
     """);
     try {
@@ -124,9 +127,10 @@ public class ExecCompilerTest {
   @Test
   public void testHalfAssignment() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     from Standard.Base.Errors.Common import all
     run value =
         x = 4
@@ -144,7 +148,8 @@ public class ExecCompilerTest {
 
   @Test
   public void redefinedArgument() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module =
+        ctx.ctx().eval(LanguageInfo.ID, """
     type My_Type
         Value a b c a
     """);
@@ -164,9 +169,10 @@ public class ExecCompilerTest {
   @Test
   public void testSelfAssignment() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     from Standard.Base.Errors.Common import all
     run value =
         meta1 = meta1
@@ -182,9 +188,10 @@ public class ExecCompilerTest {
   @Test
   public void testRecursiveDefinition() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     from Standard.Base import all
 
     run prefix =
@@ -202,8 +209,10 @@ public class ExecCompilerTest {
   @Test
   public void testDefault() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID, """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     f x=1 = x
     value_from_default =
       f default
@@ -215,9 +224,10 @@ public class ExecCompilerTest {
   @Test
   public void testIdentCalledDefault() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     f x=1 = x
     value_from_binding =
       default = 2
@@ -229,7 +239,7 @@ public class ExecCompilerTest {
 
   @Test
   public void dotUnderscore() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module = ctx.ctx().eval(LanguageInfo.ID, """
     run op =
       op._
     """);
@@ -249,9 +259,10 @@ public class ExecCompilerTest {
   @Test
   public void chainedSyntax() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     from Standard.Base import all
 
     nums n = [1, 2, 3, 4, 5]
@@ -268,7 +279,8 @@ public class ExecCompilerTest {
 
   @Test
   public void chainedSyntaxOperator() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module =
+        ctx.ctx().eval(LanguageInfo.ID, """
     nums n = n
         * 2
         % 3
@@ -281,8 +293,10 @@ public class ExecCompilerTest {
   @Test
   public void inlineReturnSignature() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID, """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     foo (x : Integer) (y : Integer) -> Integer = 10*x + y
     """);
     var foo = module.invokeMember("eval_expression", "foo");
@@ -293,9 +307,10 @@ public class ExecCompilerTest {
   @Test
   public void inlineReturnSignatureOnMemberMethod() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
             import Standard.Base.Data.Numbers
             type My_Type
                 Value x
@@ -322,7 +337,7 @@ public class ExecCompilerTest {
 
     run n = remap (Map.M n)
     """;
-    var module = ctx.eval(LanguageInfo.ID, code);
+    var module = ctx.ctx().eval(LanguageInfo.ID, code);
     var run = module.invokeMember("eval_expression", "run");
     assertTrue("run is a function", run.canExecute());
     assertEquals("(M (M 10 5) 3)", run.execute(10).toString());
@@ -331,9 +346,10 @@ public class ExecCompilerTest {
   @Test
   public void inlineReturnSignatureOnLocalFunction() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     foo x y =
         inner_foo (z : Integer) -> Integer = 100*z + 10*y + x
         a = 3
@@ -347,7 +363,7 @@ public class ExecCompilerTest {
 
   @Test
   public void inlineReturnSignatureWithoutArguments() throws Exception {
-    var module = ctx.eval(LanguageInfo.ID, """
+    var module = ctx.ctx().eval(LanguageInfo.ID, """
     the_number -> Integer = 23
     """);
     var result = module.invokeMember("eval_expression", "the_number");
@@ -374,7 +390,7 @@ public class ExecCompilerTest {
             .uri(uri)
             .buildLiteral();
 
-    var module = ctx.eval(src);
+    var module = ctx.ctx().eval(src);
     var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
     assertEquals(11, foo.execute(1).asInt());
   }
@@ -394,7 +410,7 @@ public class ExecCompilerTest {
             .buildLiteral();
 
     try {
-      var module = ctx.eval(src);
+      var module = ctx.ctx().eval(src);
       var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
       fail("Compiler error was expected, but foo evaluated successfully as: " + foo);
     } catch (PolyglotException ex) {
@@ -405,9 +421,10 @@ public class ExecCompilerTest {
   @Test
   public void testInvalidEnsoProjectRef() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     from Standard.Base.Errors.Common import all
     from Standard.Base.Meta.Enso_Project import enso_project
     run dummy =
@@ -422,9 +439,10 @@ public class ExecCompilerTest {
   @Test
   public void testDoubledRandom() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
           from Standard.Base import all
           polyglot java import java.util.Random
 
@@ -444,9 +462,10 @@ public class ExecCompilerTest {
   @Test
   public void testUnknownStaticField() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
           from Standard.Base import all
           polyglot java import java.util.Random as R
 
@@ -477,7 +496,7 @@ public class ExecCompilerTest {
       0 -> T.V
       1 -> T.V (_->N)
     """;
-    var module = ctx.eval(LanguageInfo.ID, code);
+    var module = ctx.ctx().eval(LanguageInfo.ID, code);
     var run = module.invokeMember("eval_expression", "run");
     var real = run.execute(1L);
     var realN = real.invokeMember("v");
@@ -499,7 +518,7 @@ public class ExecCompilerTest {
       read self r = r
       app fn = fn F
     """;
-    var module = ctx.eval(LanguageInfo.ID, code);
+    var module = ctx.ctx().eval(LanguageInfo.ID, code);
     var run = module.invokeMember("eval_expression", "run");
     var real = run.execute(1L);
     assertEquals("Should be the same", 1, real.asInt());
@@ -508,9 +527,10 @@ public class ExecCompilerTest {
   @Test
   public void testPropertlyIdentifyNameOfJavaClassInError() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
     from Standard.Base.Errors.Common import all
     polyglot java import java.lang.Runnable
 
@@ -531,9 +551,10 @@ public class ExecCompilerTest {
   @Test
   public void illegalPrivateConversion() throws Exception {
     var module =
-        ctx.eval(
-            LanguageInfo.ID,
-            """
+        ctx.ctx()
+            .eval(
+                LanguageInfo.ID,
+                """
         type My_Type
         type Other_Type
         private My_Type.from (other:Other_Type) =
@@ -557,7 +578,7 @@ public class ExecCompilerTest {
         First_Type.from (that:Other_Type) = 42
         run value -> First_Type = Other_Type
         """;
-    var module = ctx.eval(LanguageInfo.ID, code);
+    var module = ctx.ctx().eval(LanguageInfo.ID, code);
     var runMethod = module.invokeMember(Module.EVAL_EXPRESSION, "run");
     try {
       var r = runMethod.execute(0);
diff --git lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java
index 28f182885d..0cccc946b3 100644
--- lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java
+++ lib/java/test-utils/src/main/java/org/enso/test/utils/ContextUtils.java
@@ -8,6 +8,7 @@ import java.io.OutputStream;
 import java.nio.file.Paths;
 import java.util.Map;
 import java.util.concurrent.Callable;
+import java.util.function.Supplier;
 import java.util.logging.Level;
 import org.enso.common.LanguageInfo;
 import org.enso.common.MethodNames.Module;
@@ -20,10 +21,40 @@ import org.graalvm.polyglot.Source;
 import org.graalvm.polyglot.Value;
 import org.graalvm.polyglot.io.IOAccess;
 import org.graalvm.polyglot.proxy.ProxyExecutable;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
 
 /** A collection of classes and methods useful for testing {@link Context} related stuff. */
-public final class ContextUtils {
-  private ContextUtils() {}
+public final class ContextUtils implements TestRule {
+  private final Supplier<Context> provider;
+  private static final ThreadLocal<Context> CURRENT = new ThreadLocal<>();
+
+  public ContextUtils(Supplier<Context> lazyCtx) {
+    this.provider = lazyCtx;
+  }
+
+  @Override
+  public Statement apply(Statement base, Description description) {
+    return new Statement() {
+      @Override
+      public void evaluate() throws Throwable {
+        var prev = CURRENT.get();
+        try (var ctx = provider.get()) {
+          CURRENT.set(ctx);
+          base.evaluate();
+        } finally {
+          CURRENT.set(prev);
+        }
+      }
+    };
+  }
+
+  public final Context ctx() {
+    var ctx = CURRENT.get();
+    assert ctx != null : "Have you specified @ClassRule?";
+    return ctx;
+  }
 
   public static Context createDefaultContext() {
     var context = defaultContextBuilder().build();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-compiler p-medium Should be completed in the next few sprints
Projects
Status: New
Development

No branches or pull requests

2 participants