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

Unused import warnings #624

Open
FabijanC opened this issue Oct 9, 2024 · 0 comments
Open

Unused import warnings #624

FabijanC opened this issue Oct 9, 2024 · 0 comments
Labels
bug Something isn't working performance testing Related to code testing

Comments

@FabijanC
Copy link
Contributor

FabijanC commented Oct 9, 2024

Warning

For some time now, we've had the issue of two imports reported in mod test_unique_auto_deletable_file in crates/starknet-devnet/tests/common/utils.rs:

Our check scripts don't consider this file; it's only reported by VS Code's rust-analyzer.

Reproduce

When I run cargo check --all-targets, I get this (omitted parts are replaced with [...]):

warning: unused import: `std::path::Path`
   --> crates/starknet-devnet/benches/../tests/common/utils.rs:338:9
[...]
warning: `starknet-devnet` (bench "mint_bench") generated 2 warnings [...]

Cause

Notice how it reports the warnings as belonging to mint_benches in benches. This is due to a pub mod common annotation in mint_bench.rs:

#[path = "../tests/common/mod.rs"]

Without the mod path manipulation, the warnings are gone.

Questions

  1. Are these tests even executed in our CI? I don't see them mentioned in the logs.
  2. Are there any other tests not executed?
  3. Should we add an --all-targets check?

Mitigation

  1. Sure we could just [allow(...)] the two warnings, but I'd prefer something better.
  2. Perhaps we could just move the test_unique_auto_deletable_file module.
  3. While googling, I found this recommendation on integration test organization, which could even reduce the testing time (hence the performance label I'm putting on this issue).
  4. We could also remove the bench file, I don't think we've ever used it, and I don't think it's a part of our CI.
@FabijanC FabijanC added bug Something isn't working testing Related to code testing performance labels Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working performance testing Related to code testing
Projects
Status: 🆕 New
Development

No branches or pull requests

1 participant