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.

Sign up for GitHub

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

Jump to bottom

Optimize reference tests run and report #1402

Open
wants to merge 1 commit into
base: arith-dev
Choose a base branch
from
Open

Optimize reference tests run and report #1402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions .github/workflows/gradle-ethereum-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ jobs:
- name: Run General State Reference Tests
run: ./gradlew referenceGeneralStateTests
env:
REFERENCE_TESTS_PARALLELISM: 10
JAVA_OPTS: -Dorg.gradle.daemon=false
CORSET_FLAGS: fields,expand,expand,expand

Expand Down Expand Up @@ -101,7 +100,6 @@ jobs:
- name: Run General State Reference Tests
run: GOMEMLIMIT=196GiB ./gradlew referenceGeneralStateTests
env:
REFERENCE_TESTS_PARALLELISM: 10
JAVA_OPTS: -Dorg.gradle.daemon=false
CORSET_FLAGS: disable
GOCORSET_FLAGS: -wd --ansi-escapes=false --report --air
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/reference-blockchain-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ jobs:
timeout-minutes: 180
continue-on-error: true
env:
REFERENCE_TESTS_PARALLELISM: 20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests consume a lot of heap space so it is not feasible to have tests run in parallel as the sum of heap usage of all concurrent tests will exceed the max heap size.

JAVA_OPTS: -Dorg.gradle.daemon=false
CORSET_FLAGS: disable
GOCORSET_FLAGS: -wd --ansi-escapes=false --report --air
Expand Down
2 changes: 1 addition & 1 deletion reference-tests/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ tasks.register('referenceBlockchainTests', Test) {

boolean isCiServer = System.getenv().containsKey("CI")
minHeapSize = isCiServer ? "8g" :"4g"
maxHeapSize = isCiServer ? "50g" : "8g"
maxHeapSize = isCiServer ? "32g" : "8g"

useJUnitPlatform {
includeTags("BlockchainReferenceTest")
Expand Down
16 changes: 8 additions & 8 deletions reference-tests/src/test/resources/templates/BlockchainReferenceTest.java.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package net.consensys.linea.generated.blockchain;
import static net.consensys.linea.BlockchainReferenceTestTools.executeTest;
import static net.consensys.linea.BlockchainReferenceTestTools.generateTestParametersForConfig;

import java.util.Collection;
import java.util.concurrent.ExecutionException;
import java.util.stream.Stream;

Expand Down Expand Up @@ -49,15 +50,14 @@ public class %%TESTS_NAME%% {

public static Stream<Arguments> getTestParametersForConfig()
throws ExecutionException, InterruptedException {
if (FAILED_MODULE.isEmpty()) {
return generateTestParametersForConfig(TEST_CONFIG_FILE_DIR_PATH).stream().map(params ->
Arguments.of(params[0], params[1], params[2])
);
} else {
return generateTestParametersForConfig(TEST_CONFIG_FILE_DIR_PATH, FAILED_MODULE, FAILED_CONSTRAINT).stream().map(params ->
Arguments.of(params[0], params[1], params[2])
);
Collection<Object[]> testParameters = FAILED_MODULE.isEmpty()
? generateTestParametersForConfig(TEST_CONFIG_FILE_DIR_PATH)
: generateTestParametersForConfig(TEST_CONFIG_FILE_DIR_PATH, FAILED_MODULE, FAILED_CONSTRAINT);
Collection<Object[]> filteredTestParameters = testParameters.stream().filter(params -> (Boolean) params[2]).toList();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the test configs that were going to be skipped will be filtered here itself resulting in a cleaner test report without 10s of thousands of skipped tests. I still need to ensure if all test configs are filtered there is atleast one dummy test config returned. This is because junit expects atleast 1 set of parameters for parameterized test.

if (filteredTestParameters.isEmpty()) {
return Stream.of(Arguments.of("%%TESTS_NAME%%", null, false));
}
return filteredTestParameters.stream().map(params -> Arguments.of(params[0], params[1], params[2]));
}

@ParameterizedTest(name = "Name: {0}")
Expand Down
11 changes: 8 additions & 3 deletions reference-tests/src/test/resources/templates/GeneralStateReferenceTest.java.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import static net.consensys.linea.GeneralStateReferenceTestTools.executeTest;
import static net.consensys.linea.GeneralStateReferenceTestTools.generateTestParametersForConfig;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import java.util.Collection;
import java.util.stream.Stream;

import org.hyperledger.besu.ethereum.referencetests.GeneralStateTestCaseEipSpec;
Expand All @@ -39,9 +40,13 @@ public class %%TESTS_NAME%% {
private static final String[] TEST_CONFIG_FILE_DIR_PATH = new String[] {%%TESTS_FILE%%};

public static Stream<Arguments> getTestParametersForConfig() {
return generateTestParametersForConfig(TEST_CONFIG_FILE_DIR_PATH).stream().map(params ->
Arguments.of(params[0], params[1], params[2])
);
Collection<Object[]> testParameters = generateTestParametersForConfig(TEST_CONFIG_FILE_DIR_PATH)
.stream().filter(params -> (Boolean) params[2]).toList();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that the test configs that were going to be skipped will be filtered here itself resulting in a cleaner test report without 10s of thousands of skipped tests. I still need to ensure if all test configs are filtered there is atleast one dummy test config returned. This is because junit expects atleast 1 set of parameters for parameterized test.

if (testParameters.isEmpty()) {
return Stream.of(Arguments.of("%%TESTS_NAME%%", null, false));
} else {
return testParameters.stream().map(params -> Arguments.of(params[0], params[1], params[2]));
}
}

@ParameterizedTest(name = "Name: {0}")
Expand Down
Loading