Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: more fixes for --all-features tests #8946

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

grandizzy
Copy link
Collaborator

Motivation

more fixes for cargo test --all --all-features clean run

  • send shutdown signal for long running tests that spawns anvil, fixing
failed to spawn node: Os { code: 98, kind: AddrInUse, message: "Address already in use" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  • avoid RPC URL rate limit for storage test
  • avoid etherscan rate limit for verify_bytecode tests, fixing
Error: 
Received error response: status=0,message=NOTOK, result=Some("Max calls per sec rate limit reached (5/sec)")
failures:
    verify_bytecode::can_verify_bytecode_with_metadata
  • remove DAPP_REMAPPINGS env var from can_override_config test, fixing
thread 'config::can_override_config' panicked at crates/forge/tests/cli/config.rs:210:5:
assertion failed: `(left == right)`'
  left: `"ds-test/=/tmp/can_override_config-68Zsmm2o/lib/forge-std/lib/ds-test/from-env/"`
 right: `"ds-test/=/tmp/can_override_config-68Zsmm2o/lib/forge-std/lib/ds-test/from-file/"`

Differences (-left|+right):
-ds-test/=/tmp/can_override_config-68Zsmm2o/lib/forge-std/lib/ds-test/from-env/
+ds-test/=/tmp/can_override_config-68Zsmm2o/lib/forge-std/lib/ds-test/from-file/

Solution

@grandizzy grandizzy marked this pull request as ready for review September 24, 2024 09:59
@grandizzy grandizzy enabled auto-merge (squash) September 24, 2024 09:59
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I see, that makes sense

one suggestion

Comment on lines +796 to +798
if let Some(signal) = handle.shutdown_signal_mut().take() {
signal.fire().unwrap();
}
Copy link
Member

Choose a reason for hiding this comment

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

could we convert this to a function on handle?

and document why we call this for some tests

@grandizzy grandizzy merged commit 232e6c7 into foundry-rs:master Sep 24, 2024
20 checks passed
@@ -205,6 +205,7 @@ forgetest_init!(can_override_config, |prj, cmd| {
);

// env vars work
std::env::remove_var("DAPP_REMAPPINGS");
Copy link
Member

Choose a reason for hiding this comment

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

we should just not be using env vars in cli tests, it can only work if its set on a spawned command

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, will clean those tests and move such remapping test in dedicated

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants