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

Lockfile tests failing #10567

Open
1 task done
emnul opened this issue Aug 27, 2024 · 7 comments
Open
1 task done

Lockfile tests failing #10567

emnul opened this issue Aug 27, 2024 · 7 comments
Labels
A-db Related to the database C-bug An unexpected or incorrect behavior C-test A change that impacts how or what we test

Comments

@emnul
Copy link
Contributor

emnul commented Aug 27, 2024

Describe the bug

The following tests in lockfile::tests::test_drop_lock and lockfile::tests::test_lock in crates/storage/db/src/lockfile.rs are failing.

Steps to reproduce

Run make test on main branch. Observe the following error:

failures:

---- lockfile::tests::test_drop_lock stdout ----
thread 'lockfile::tests::test_drop_lock' panicked at crates/storage/db/src/lockfile.rs:198:9:
assertion failed: lock_file.exists()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- lockfile::tests::test_lock stdout ----
thread 'lockfile::tests::test_lock' panicked at crates/storage/db/src/lockfile.rs:153:54:
called `Result::unwrap()` on an `Err` value: PoisonError { .. }


failures:
    lockfile::tests::test_drop_lock
    lockfile::tests::test_lock

test result: FAILED. 39 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.65s

Node logs

No response

Platform(s)

Mac (Apple Silicon)

What version/commit are you on?

1.0.5-dev

What database version are you on?

N/A

Which chain / network are you on?

N/A

What type of node are you running?

Archive (default)

What prune config do you use, if any?

No response

If you've built Reth from source, provide the full command you used

No response

Code of Conduct

  • I agree to follow the Code of Conduct
@emnul emnul added C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled labels Aug 27, 2024
@tcoratger
Copy link
Contributor

I think, at least for test_drop_lock , this is because of the following line:

// Too expensive for ef-tests to write/read lock to/from disk.
Ok(Self(Arc::new(StorageLockInner { file_path })))

as we don't use

Ok(Self(Arc::new(StorageLockInner::new(file_path)?)))

I imagine because of the comment (ef tests overload) so that it does not creates the lock file/writes into it.

@nkysg
Copy link
Contributor

nkysg commented Aug 29, 2024

the same issue.

@emhane emhane added A-db Related to the database and removed S-needs-triage This issue needs to be labelled labels Aug 30, 2024
@emhane
Copy link
Member

emhane commented Aug 30, 2024

seems like locally there is issues with clean up, possibly also with concurrency running different tests @Rjected

@emhane emhane added the C-test A change that impacts how or what we test label Aug 30, 2024
@emhane
Copy link
Member

emhane commented Aug 30, 2024

we should probably re-write the make test command to run nextest instead @mattsse ? ref #9403

Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Sep 21, 2024
@emhane emhane removed the S-stale This issue/PR is stale and will close with no further activity label Sep 23, 2024
@Thegaram
Copy link
Contributor

It seems that the issue is related to this PR: #8753

With this change in testing/ef-tests/Cargo.toml cargo test --workspace passes:

- reth-db = { workspace = true, features = ["mdbx", "test-utils", "disable-lock"] }
+ reth-db = { workspace = true, features = ["mdbx", "test-utils"] }

@nkysg
Copy link
Contributor

nkysg commented Sep 28, 2024

It seems that the issue is related to this PR: #8753

With this change in testing/ef-tests/Cargo.toml cargo test --workspace passes:

- reth-db = { workspace = true, features = ["mdbx", "test-utils", "disable-lock"] }
+ reth-db = { workspace = true, features = ["mdbx", "test-utils"] }

@emhane has a pr #10636, but I don't know why she closed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-db Related to the database C-bug An unexpected or incorrect behavior C-test A change that impacts how or what we test
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

6 participants