Parallel execution of round trip tests #1845

oowekyala · 2023-06-15T10:39:58Z

The formatter round trip tests currently take about a minute on my machine. Using concurrent execution and a small refactoring this is now down to under 20secs.

I converted the test to use Junit dynamic tests, which means each individual test file gets its own test instance. This allows JUnit to parallelize them, and the IDE to display each test and its status separately. Additionally, the test is linked to the source file, so that if you click on it from the IDE you'll jump to the test file.

oowekyala · 2023-06-15T10:41:53Z

core/build.gradle

@@ -14,7 +14,10 @@ sourceSets {
    }
    test {
        java {
-            srcDirs = ['src/test/java', 'test-gen']
+            srcDirs = ['src/test/java']


I changed this because both testFixtures and test used test-gen as a same source directory. Intellij does not support this. Since test depends on testFixtures there is no problem.

Oh, that explains why Intellij showed all these errors. Thanks!

I just noticed that test-gen is also removed from the test fixtures. Now I am wondering: why does this work?

Oh, it works because the PR adds a custom implementation of LfParsingTestHelper, which is the only class generated in test-gen.

Yes! I don't know if we can disable the generation of the test-gen directory entirely, but at least it is ignored

In fact, we can disable it. See #1858

gradle/wrapper/gradle-wrapper.properties

cmnrd

Cool! This makes a lot of sense. We'll have to convince the CI as well though ;)

lhstrh · 2023-06-15T17:16:46Z

This is what CI complains about:

RoundTripTests > roundTripTestFactory() > initializationError FAILED
    java.lang.ExceptionInInitializerError
        at org.lflang.tests.compiler.RoundTripTests.roundTripTestFactory(RoundTripTests.java:44)

        Caused by:
        java.lang.RuntimeException: Cannot create a resource for 'file:/home/runner/work/lingua-franca/lingua-franca/test/C/src/PhysicalConnection.lf'; a registered resource factory is needed
            at org.eclipse.xtext.resource.XtextResourceSet.getResource(XtextResourceSet.java:263)
            at org.eclipse.xtext.resource.SynchronizedXtextResourceSet.getResource(SynchronizedXtextResourceSet.java:33)
            at org.lflang.tests.TestRegistry$TestDirVisitor.visitFile(TestRegistry.java:360)
            at org.lflang.tests.TestRegistry$TestDirVisitor.visitFile(TestRegistry.java:297)
            at java.base/java.nio.file.Files.walkFileTree(Files.java:2811)
            at java.base/java.nio.file.Files.walkFileTree(Files.java:2882)
            at org.lflang.tests.TestRegistry$TestDirVisitor.walk(TestRegistry.java:381)
            at org.lflang.tests.TestRegistry.<clinit>(TestRegistry.java:205)
            ... 1 more

Make TestRegistry a non-static singleton that can be injected.

core/src/test/java/org/lflang/tests/compiler/LinguaFrancaValidationTest.java

core/src/testFixtures/java/org/lflang/tests/TestRegistry.java

oowekyala · 2023-06-16T14:34:17Z

Maybe this Junit feature can be used in the future to run the target tests concurrently, which would greatly speed them up as they spend large amounts of time waiting on IO.

Several versions of org.eclipse.core.runtime were being provided to the integrationTest module, which made IntelliJ fail with a SecurityException

lhstrh · 2023-06-17T01:00:18Z

buildSrc/src/main/groovy/org.lflang.test-conventions.gradle

@@ -21,7 +21,7 @@ dependencies {
    testFixturesImplementation "org.junit.platform:junit-platform-commons:$jUnitPlatformVersion"
    testFixturesImplementation "org.junit.platform:junit-platform-engine:$jUnitPlatformVersion"
    testFixturesImplementation "org.opentest4j:opentest4j:$openTest4jVersion"
-    testFixturesImplementation "org.eclipse.xtext:org.eclipse.xtext.testing:$xtextVersion"
+    testFixturesImplementation platform("org.eclipse.xtext:org.eclipse.xtext.testing:$xtextVersion")


What does this magic incantation do?

I don't understand it well, but it changes the version conflict resolution for dependencies. Previously, running tests from the integrationTest module in intellij failed with

java.lang.SecurityException: class "org.eclipse.core.runtime.OperationCanceledException"'s signer information does not match signer information of other classes in the same package

which indicates there are several jars on the classpath containing classes in this package --dependency hell.
The platform incantation forces the version conflict resolver to pick the dependency versions required by the 'platform', ie the versions it has been built and tested with. Without it, gradle is free to pick a newer version of each dependency independently, which resulted in an inconsistent dependency set apparently.

You can inspect how this affects our dependencies by running ./gradle :core:dependencies --configuration integrationTestRuntimeClasspath

Interesting.. Thanks for the fix and explaining this!!

lhstrh

This looks great!

core/src/testFixtures/java/org/lflang/tests/LFInjectorProvider.java

….java

oowekyala · 2023-06-17T13:06:50Z

Thanks for merging!

oowekyala added 2 commits June 14, 2023 17:38

Use dynamic tests

9c19846

Reuse injector for parsing

fddbfbc

oowekyala commented Jun 15, 2023

View reviewed changes

Merge branch 'master' into clem.dynamic-tests

20371e0

oowekyala force-pushed the clem.dynamic-tests branch from 578fe42 to 20371e0 Compare June 15, 2023 10:42

oowekyala commented Jun 15, 2023

View reviewed changes

gradle/wrapper/gradle-wrapper.properties Show resolved Hide resolved

Format

628fb95

cmnrd approved these changes Jun 15, 2023

View reviewed changes

oowekyala added 3 commits June 16, 2023 15:27

Make LfInjectorProvider a singleton

9fd7187

Make TestRegistry a non-static singleton that can be injected.

Also use dynamic tests for target property checks

fddce8e

Format

dcec369

oowekyala commented Jun 16, 2023

View reviewed changes

core/src/test/java/org/lflang/tests/compiler/LinguaFrancaValidationTest.java Show resolved Hide resolved

oowekyala commented Jun 16, 2023

View reviewed changes

core/src/testFixtures/java/org/lflang/tests/TestRegistry.java Show resolved Hide resolved

oowekyala added 2 commits June 16, 2023 16:25

Fix LspTests

e86c44e

Remove dependency on xtext.testing

c6d43af

Fix dependency problem

de0d534

Several versions of org.eclipse.core.runtime were being provided to the integrationTest module, which made IntelliJ fail with a SecurityException

lhstrh reviewed Jun 17, 2023

View reviewed changes

lhstrh approved these changes Jun 17, 2023

View reviewed changes

core/src/testFixtures/java/org/lflang/tests/LFInjectorProvider.java Outdated Show resolved Hide resolved

Update core/src/testFixtures/java/org/lflang/tests/LFInjectorProvider…

68b29ac

….java

lhstrh enabled auto-merge June 17, 2023 01:08

lhstrh added testing enhancement Enhancement of existing feature labels Jun 17, 2023

lhstrh changed the title ~~Use dynamic tests for formatter round trip tests~~ Parallel execution of round trip tests Jun 17, 2023

lhstrh added 2 commits June 16, 2023 21:07

Merge remote-tracking branch 'base/master' into clem.dynamic-tests

9913e73

Fix formatting

5a13ed9

lhstrh mentioned this pull request Jun 17, 2023

Add validator checks for filesystem paths #1852

Open

lhstrh added this pull request to the merge queue Jun 17, 2023

Merged via the queue into lf-lang:master with commit 57e803f Jun 17, 2023

oowekyala deleted the clem.dynamic-tests branch June 17, 2023 12:33

petervdonovan mentioned this pull request Jun 18, 2023

Allow tests to be run concurrently #1854

Open

cmnrd mentioned this pull request Jun 19, 2023

Cleanup gradle config #1858

Merged

petervdonovan removed the enhancement Enhancement of existing feature label Aug 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parallel execution of round trip tests #1845

Parallel execution of round trip tests #1845

oowekyala commented Jun 15, 2023

oowekyala Jun 15, 2023

cmnrd Jun 15, 2023

cmnrd Jun 19, 2023

cmnrd Jun 19, 2023

oowekyala Jun 19, 2023

cmnrd Jun 19, 2023

cmnrd left a comment •

edited

Loading

lhstrh commented Jun 15, 2023

oowekyala commented Jun 16, 2023

lhstrh Jun 17, 2023

oowekyala Jun 17, 2023

lhstrh Jun 17, 2023

lhstrh left a comment

oowekyala commented Jun 17, 2023

Parallel execution of round trip tests #1845

Parallel execution of round trip tests #1845

Conversation

oowekyala commented Jun 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmnrd left a comment • edited Loading

Choose a reason for hiding this comment

lhstrh commented Jun 15, 2023

oowekyala commented Jun 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhstrh left a comment

Choose a reason for hiding this comment

oowekyala commented Jun 17, 2023

cmnrd left a comment •

edited

Loading