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

Refactoring to use test watcher for unit tests as well. #1513

Merged
merged 32 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
e2e9f1a
Refactoring to use test watcher for unit tests as well.
FlorianHuc Nov 19, 2024
490c699
add header
FlorianHuc Nov 19, 2024
c31d933
spotless apply
FlorianHuc Nov 19, 2024
40d88d7
Merge remote-tracking branch 'origin/arith-dev' into feat/watch-unit-…
FlorianHuc Nov 20, 2024
d21c629
skip refTest because of balance too high
letypequividelespoubelles Nov 20, 2024
86b65fd
skip more huge balance tests
letypequividelespoubelles Nov 20, 2024
4ffb909
Merge branch 'arith-dev' into feat/watch-unit-tests
letypequividelespoubelles Nov 20, 2024
7a2029c
Merge remote-tracking branch 'origin/arith-dev' into feat/watch-unit-…
FlorianHuc Nov 20, 2024
ba9edf2
add watcher to all tests.
FlorianHuc Nov 21, 2024
26f56b4
merge
FlorianHuc Nov 25, 2024
05ce87a
fix merge
FlorianHuc Nov 25, 2024
e1495c1
fix merge
FlorianHuc Nov 25, 2024
060a1f7
revert due to performance regression
FlorianHuc Nov 25, 2024
e5fbebe
skip more test
letypequividelespoubelles Nov 25, 2024
8bed792
one more
letypequividelespoubelles Nov 25, 2024
72beac7
Merge remote-tracking branch 'origin/arith-dev' into feat/watch-unit-…
FlorianHuc Nov 26, 2024
f9c02a5
Merge remote-tracking branch 'origin/feat/watch-unit-tests' into feat…
FlorianHuc Nov 26, 2024
4b375a3
Merge branch 'arith-dev' into feat/watch-unit-tests
FlorianHuc Nov 26, 2024
e46371c
update linea-constraints
FlorianHuc Nov 26, 2024
aeb9779
Merge branch 'arith-dev' into feat/watch-unit-tests
FlorianHuc Nov 26, 2024
f8af05b
fix: increment failure counter + shorter error message
OlivierBBB Nov 26, 2024
96aa0be
spotless
OlivierBBB Nov 26, 2024
5e0c27e
Merge branch 'arith-dev' into feat/watch-unit-tests
OlivierBBB Nov 26, 2024
ebb23e9
testing mem issue
FlorianHuc Nov 27, 2024
61c2b9a
Merge remote-tracking branch 'origin/feat/watch-unit-tests' into feat…
FlorianHuc Nov 27, 2024
dafe2c2
remove tests watcher
FlorianHuc Nov 27, 2024
46a3ddf
spotless
FlorianHuc Nov 27, 2024
a4129d3
spotless
FlorianHuc Nov 27, 2024
ec73621
Merge branch 'arith-dev' into feat/watch-unit-tests
FlorianHuc Nov 27, 2024
0905873
Merge remote-tracking branch 'origin/arith-dev' into feat/watch-unit-…
FlorianHuc Nov 27, 2024
ae697b6
add back deleted test
FlorianHuc Nov 27, 2024
40b16a0
remove double brackets.
FlorianHuc Nov 27, 2024
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
1 change: 1 addition & 0 deletions arithmetization/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ dependencies {
testImplementation "${besuArtifactGroup}.internal:rlp"
testImplementation "${besuArtifactGroup}.internal:core"
testImplementation "${besuArtifactGroup}.internal:referencetests"
testImplementation 'org.junit.platform:junit-platform-launcher'
}

configurations {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Consensys Software Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package net.consensys.linea;

import static net.consensys.linea.reporting.TestOutcomeWriterTool.writeToJsonFile;

import org.junit.platform.launcher.LauncherSession;
import org.junit.platform.launcher.LauncherSessionListener;

public class UnitTestOutcomeWriter implements LauncherSessionListener {

public static final String FILE_NAME = "UnitTestsResults.json";

@Override
public void launcherSessionClosed(LauncherSession session) {
writeToJsonFile(FILE_NAME);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright Consensys Software Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package net.consensys.linea;

import java.util.Optional;

import lombok.extern.slf4j.Slf4j;
import net.consensys.linea.reporting.TestOutcomeWriterTool;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.TestWatcher;

@Slf4j
public class UnitTestWatcher implements TestWatcher {

private String FAILED = "FAILED";

@Override
public void testFailed(ExtensionContext context, Throwable cause) {
String testName = context.getDisplayName();
log.info("Adding failure for {}", testName);
TestOutcomeWriterTool.addFailure(
FAILED, cause.getMessage().split(System.lineSeparator(), 2)[0], testName);
log.info("Failure added for {}", testName);
}

@Override
public void testSuccessful(ExtensionContext context) {
TestOutcomeWriterTool.addSuccess();
}

@Override
public void testDisabled(ExtensionContext context, Optional<String> reason) {
TestOutcomeWriterTool.addSkipped();
}

@Override
public void testAborted(ExtensionContext context, Throwable cause) {
TestOutcomeWriterTool.addAborted();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
// A TestWatcher is used to log the results of testEcPairingSingleForScenario
// into a csv file (one for successful and one for failing cases)
// that can be used to run the same test cases with @CsvFileSource

@ExtendWith(EcPairingTestWatcher.class)
public class EcPairingrTest {
// https://github.com/Consensys/linea-arithmetization/issues/822
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.junit.jupiter.api.parallel.ExecutionMode;

// https://github.com/Consensys/linea-besu-plugin/issues/197

@Execution(ExecutionMode.SAME_THREAD)
public class MxpTest {

Expand Down
2 changes: 1 addition & 1 deletion reference-tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,6 @@ dependencies {

implementation project(":arithmetization")
implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml'
testImplementation project(path: ':testing')
implementation project(path: ':testing')
implementation 'org.junit.platform:junit-platform-launcher'
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
package net.consensys.linea;

import static net.consensys.linea.ReferenceTestOutcomeRecorderTool.setFileDirectory;
import static net.consensys.linea.reporting.TestOutcomeWriterTool.getFileDirectory;

import java.io.IOException;
import java.nio.file.FileAlreadyExistsException;
Expand All @@ -31,7 +31,7 @@ public class BlockchainReferenceTestJson {

@Synchronized
public static CompletableFuture<String> readBlockchainReferenceTestsOutput(String fileName) {
String fileDirectory = setFileDirectory();
String fileDirectory = getFileDirectory();
return CompletableFuture.supplyAsync(
() -> {
Path directoryPath = Paths.get(fileDirectory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,16 @@
*/
package net.consensys.linea;

import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import static net.consensys.linea.reporting.TestOutcomeWriterTool.addFailure;

import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListSet;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.Synchronized;
import lombok.extern.slf4j.Slf4j;
import net.consensys.linea.zktracer.json.JsonConverter;
import net.consensys.linea.reporting.TestOutcomeWriterTool;
import net.consensys.linea.reporting.TestState;

@Slf4j
public class ReferenceTestOutcomeRecorderTool {
Expand All @@ -42,51 +34,23 @@ public class ReferenceTestOutcomeRecorderTool {
public static final String JSON_OUTPUT_FILENAME =
System.getenv()
.getOrDefault("REFERENCE_TEST_OUTCOME_OUTPUT_FILE", "failedReferenceTests.json");
public static JsonConverter jsonConverter = JsonConverter.builder().build();
private static volatile AtomicInteger failedCounter = new AtomicInteger(0);
private static volatile AtomicInteger successCounter = new AtomicInteger(0);
private static volatile AtomicInteger disabledCounter = new AtomicInteger(0);
private static volatile AtomicInteger abortedCounter = new AtomicInteger(0);
private static volatile ConcurrentMap<
String, ConcurrentMap<String, ConcurrentSkipListSet<String>>>
modulesToConstraintsToTests = new ConcurrentHashMap<>();

public static void mapAndStoreTestResult(
String testName, TestState success, Map<String, Set<String>> failedConstraints) {
switch (success) {
case FAILED -> {
failedCounter.incrementAndGet();
TestOutcomeWriterTool.addFailed();
for (Map.Entry<String, Set<String>> failedConstraint : failedConstraints.entrySet()) {
String moduleName = failedConstraint.getKey();
for (String constraint : failedConstraint.getValue()) {
ConcurrentMap<String, ConcurrentSkipListSet<String>> constraintsToTests =
modulesToConstraintsToTests.computeIfAbsent(
moduleName, m -> new ConcurrentHashMap<>());
ConcurrentSkipListSet<String> failingTests =
constraintsToTests.computeIfAbsent(constraint, m -> new ConcurrentSkipListSet<>());
int size = failingTests.size();
failingTests.add(testName);
if (failingTests.size() == size) {
log.warn("Duplicate name found... {}", failedConstraint);
}
addFailure(moduleName, constraint, testName);
}
}
}
case SUCCESS -> successCounter.incrementAndGet();
case ABORTED -> abortedCounter.incrementAndGet();
case DISABLED -> disabledCounter.incrementAndGet();
}
}

@Synchronized
public static BlockchainReferenceTestOutcome parseBlockchainReferenceTestOutcome(
String jsonString) {
if (!jsonString.isEmpty()) {
BlockchainReferenceTestOutcome blockchainReferenceTestOutcome =
jsonConverter.fromJson(jsonString, BlockchainReferenceTestOutcome.class);
return blockchainReferenceTestOutcome;
case SUCCESS -> TestOutcomeWriterTool.addSuccess();
case ABORTED -> TestOutcomeWriterTool.addAborted();
case DISABLED -> TestOutcomeWriterTool.addSkipped();
}
throw new RuntimeException("invalid JSON");
}

public static Map<String, Set<String>> extractConstraints(String message) {
Expand Down Expand Up @@ -174,57 +138,5 @@ private static void getPairFromString(String constraint, Map<String, Set<String>
pairs.computeIfAbsent(pair[0].trim(), p -> new HashSet<>()).add(pair[1].trim());
}

@Synchronized
public static void writeToJsonFile() {
try {
String directory = setFileDirectory();
log.info("Reference test will be written to file {} \\ {}", directory, JSON_OUTPUT_FILENAME);
writeToJsonFileInternal(JSON_OUTPUT_FILENAME).get();
log.info("Reference test results written to file {}", JSON_OUTPUT_FILENAME);
log.info(
"Path exists: {}, file exist: {}",
Paths.get(directory).toFile().exists(),
Paths.get(directory).resolve(JSON_OUTPUT_FILENAME).toFile().exists());
} catch (Exception e) {
log.error("Error while writing results");
throw new RuntimeException("Error while writing results", e);
}
}

static ObjectMapper objectMapper = new ObjectMapper();

@Synchronized
private static CompletableFuture<Void> writeToJsonFileInternal(String name) {
String fileDirectory = setFileDirectory();
log.info("writing results summary to {}", fileDirectory + "/" + name);
try {
Files.createDirectories(Path.of(fileDirectory));
} catch (IOException e) {
log.error("Error - Failed to create test directory output: %s".formatted(e.getMessage()));
throw new RuntimeException(e);
}
return CompletableFuture.runAsync(
() -> {
try (FileWriter file = new FileWriter(Path.of(fileDirectory, name).toString())) {
objectMapper.writeValue(
file,
new BlockchainReferenceTestOutcome(
failedCounter.get(),
successCounter.get(),
disabledCounter.get(),
abortedCounter.get(),
modulesToConstraintsToTests));
} catch (Exception e) {
log.error("Error - Failed to write test output: %s".formatted(e.getMessage()));
}
});
}

static String setFileDirectory() {
String jsonDirectory = System.getenv("FAILED_TEST_JSON_DIRECTORY");
if (jsonDirectory == null || jsonDirectory.isEmpty()) {
return "../tmp/local/";
}
return jsonDirectory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static net.consensys.linea.ReferenceTestOutcomeRecorderTool.*;

import lombok.extern.slf4j.Slf4j;
import net.consensys.linea.reporting.TestOutcomeWriterTool;
import org.junit.platform.launcher.LauncherSession;
import org.junit.platform.launcher.LauncherSessionListener;

Expand All @@ -25,6 +26,6 @@ public class ReferenceTestOutcomeWriter implements LauncherSessionListener {

@Override
public void launcherSessionClosed(LauncherSession session) {
writeToJsonFile();
TestOutcomeWriterTool.writeToJsonFile(JSON_OUTPUT_FILENAME);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@

import lombok.extern.slf4j.Slf4j;
import net.consensys.linea.corset.CorsetValidator;
import net.consensys.linea.reporting.TestOutcome;
import net.consensys.linea.reporting.TestOutcomeWriterTool;
import net.consensys.linea.testing.ExecutionEnvironment;
import net.consensys.linea.zktracer.ZkTracer;
import org.hyperledger.besu.ethereum.MainnetBlockValidator;
Expand Down Expand Up @@ -94,6 +96,18 @@ public class BlockchainReferenceTestTools {
PARAMS.ignore("RevertInCreateInInitCreate2_d0g0v0_London[London]");
PARAMS.ignore("RevertInCreateInInit_d0g0v0_London[London]");

// Arithmetization restriction: recipient address is a precompile.
PARAMS.ignore("modexpRandomInput_d0g0v0_London[London]");
PARAMS.ignore("modexpRandomInput_d0g1v0_London[London]");
PARAMS.ignore("modexpRandomInput_d1g0v0_London[London]");
PARAMS.ignore("modexpRandomInput_d1g1v0_London[London]");
PARAMS.ignore("modexpRandomInput_d2g0v0_London[London]");
PARAMS.ignore("modexpRandomInput_d2g1v0_London[London]");
PARAMS.ignore("randomStatetest642_d0g0v0_London[London]");
PARAMS.ignore("randomStatetest644_d0g0v0_London[London]");
PARAMS.ignore("randomStatetest645_d0g0v0_London[London]");
PARAMS.ignore("randomStatetest645_d0g0v1_London[London]");

// Consumes a huge amount of memory.
PARAMS.ignore("static_Call1MB1024Calldepth_d1g0v0_\\w+");
PARAMS.ignore("ShanghaiLove_.*");
Expand All @@ -105,6 +119,16 @@ public class BlockchainReferenceTestTools {
// Absurd amount of gas, doesn't run in parallel.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I imagine that the CALL50000 could prove problematic with the MemoryRange implementation we currently have @letypequividelespoubelles ^^ If we have to keep 50k snapshots of memory ^^ I mean, it's not an issue. But I wonder whether the tests that @amkCha was able to run again on that 64GB machine would crash with our current tracer.

PARAMS.ignore("randomStatetest94_\\w+");

// Balance is more than 128 bits
PARAMS.ignore("Call1024PreCalls_d0g0v0_London[London]");
PARAMS.ignore("Call1024PreCalls_d0g1v0_London[London]");
PARAMS.ignore("OverflowGasRequire_London[London]");
PARAMS.ignore("StrangeContractCreation_London[London]");
PARAMS.ignore("SuicideIssue_London[London]");
PARAMS.ignore("DelegateCallSpam_London[London]");
PARAMS.ignore("OverflowGasRequire2_d0g0v0_London[London]");
PARAMS.ignore("HighGasLimit_d0g0v0_London[London]");

FlorianHuc marked this conversation as resolved.
Show resolved Hide resolved
// Don't do time-consuming tests.
PARAMS.ignore("CALLBlake2f_MaxRounds.*");
PARAMS.ignore("loopMul_*");
Expand All @@ -129,9 +153,9 @@ public static CompletableFuture<Set<String>> getRecordedFailedTestsFromJson(
return CompletableFuture.completedFuture(failedTests);
}

CompletableFuture<BlockchainReferenceTestOutcome> modulesToConstraintsFutures =
CompletableFuture<TestOutcome> modulesToConstraintsFutures =
readBlockchainReferenceTestsOutput(JSON_INPUT_FILENAME)
.thenApply(ReferenceTestOutcomeRecorderTool::parseBlockchainReferenceTestOutcome);
.thenApply(TestOutcomeWriterTool::parseTestOutcome);

return modulesToConstraintsFutures.thenApply(
blockchainReferenceTestOutcome -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListSet;

import net.consensys.linea.reporting.TestOutcome;
import net.consensys.linea.reporting.TestOutcomeWriterTool;
import net.consensys.linea.reporting.TestState;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -52,13 +55,13 @@ void multipleModulesAreStoredCorrectly() {
"test1", TestState.FAILED, Map.of("Constraint", Set.of(module1, module2)));
ReferenceTestOutcomeRecorderTool.mapAndStoreTestResult(
"test2", TestState.FAILED, Map.of("Constraint", Set.of(module1)));
ReferenceTestOutcomeRecorderTool.writeToJsonFile();
TestOutcomeWriterTool.writeToJsonFile(JSON_OUTPUT_FILENAME_TEST);

readBlockchainReferenceTestsOutput(JSON_OUTPUT_FILENAME_TEST)
.thenApply(
jsonString -> {
BlockchainReferenceTestOutcome blockchainReferenceTestOutcome =
ReferenceTestOutcomeRecorderTool.parseBlockchainReferenceTestOutcome(jsonString);
TestOutcome blockchainReferenceTestOutcome =
TestOutcomeWriterTool.parseTestOutcome(jsonString);

ConcurrentMap<String, ConcurrentMap<String, ConcurrentSkipListSet<String>>>
modulesToConstraints =
Expand Down Expand Up @@ -155,8 +158,8 @@ public void extractConstraints() {

@Test
void parseBlockchainReferenceTestOutcome() {
BlockchainReferenceTestOutcome outcome =
ReferenceTestOutcomeRecorderTool.parseBlockchainReferenceTestOutcome(
TestOutcome outcome =
TestOutcomeWriterTool.parseTestOutcome(
"""
{
"abortedCounter": 20,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
package net.consensys.linea;

import static net.consensys.linea.TestState.*;
import static net.consensys.linea.reporting.TestState.*;
import static net.consensys.linea.testing.ExecutionEnvironment.CORSET_VALIDATION_RESULT;

import java.util.HashMap;
Expand Down
Loading
Loading