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

Conversation

FlorianHuc
Copy link
Collaborator

No description provided.

@letypequividelespoubelles letypequividelespoubelles linked an issue Nov 25, 2024 that may be closed by this pull request
@OlivierBBB OlivierBBB self-requested a review November 27, 2024 15:51
@@ -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.

}

public static void addSkipped() {
disabledCounter.incrementAndGet();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's strange that we talk about disabled and skipped rather than having one word for both. Presumably disabled makes the most sense if it in any way involves the @Disabled annotation (which I have no idea if it does or not)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it comes from the interface i implement. So I guess it's standard.

Copy link
Collaborator

@OlivierBBB OlivierBBB left a comment

Choose a reason for hiding this comment

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

Modulo the deletion of a test file which I don't know if it was intentional and my limited understanding of other parts, it looks good to me.

@@ -105,6 +119,16 @@ public class BlockchainReferenceTestTools {
// Absurd amount of gas, doesn't run in parallel.
PARAMS.ignore("randomStatetest94_\\w+");

// Balance is more than 128 bits
PARAMS.ignore(("Call1024PreCalls_d0g0v0_London[London]"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it normal to have double bracket @FlorianHuc ?

@FlorianHuc FlorianHuc enabled auto-merge (squash) November 27, 2024 17:40
@FlorianHuc FlorianHuc merged commit 120e98c into arith-dev Nov 27, 2024
5 checks passed
@FlorianHuc FlorianHuc deleted the feat/watch-unit-tests branch November 27, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

skip reference test with too big values
3 participants