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.

Sign up for GitHub

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

Jump to bottom

fix: addressed lockfile and tempdir issues #57

Open
wants to merge 1,557 commits into
base: main
Choose a base branch
from
Open

fix: addressed lockfile and tempdir issues #57

wants to merge 1,557 commits into from

Conversation

0xObsidian
Copy link

@0xObsidian 0xObsidian commented Nov 28, 2024 •
edited
Loading

Two issues were causing static file test failures:

  1. Missing lockfile in test file count
  2. Premature cleanup of temporary test directories

Bug Description

  1. File Count Mismatch:

    • Tests expected 10 files (9 static files + 1 lockfile)
    • Only had 9 files because lockfile wasn't created
    • Led to assertion failures in file count checks

  2. Directory Cleanup Race:

    • TempDir was being dropped too early due to pattern matching: 
      let (static_dir, _) = create_test_static_files_dir();
    • Caused test directory to be cleaned up while tests were still running
    • Worked in isolation but failed in full test suite

Reproduction

Pull the previous commit and run from project root:

make test-unit

Fixes made

  • Added lockfile creation before static file operations
  • Fixed file count assertions
  • Fixed premature cleanup of test directories

Testing the fix

Pull this commit and run the test suite:

make test-unit

tcoratger and others added 30 commits November 15, 2024 09:41
@tcoratger
fmt(primitives): group pub use transaction (paradigmxyz#12561)
1aa316e
@tcoratger
net: use BlockWithParent in SyncTarget::Gap (paradigmxyz#12514)
93ec6d4
@tcoratger
engine-primitives: make engine-tree independent of beacon-consensus c…
44964ac 
…rate (paradigmxyz#12560)
@mattsse
chore: extract witness recorder helper type (paradigmxyz#12566)
cd9da55
@klkvr
feat: make StaticFileProvider generic over NodePrimitives (paradi…
72a52d5 
…gmxyz#12565)
@mattsse @onbjerg
feat: introduce debug witness api (paradigmxyz#12567)
305a1ce 
Co-authored-by: Oliver <[email protected]>
@mattsse
ci: exclude more crates for op tests (paradigmxyz#12568)
efa350d
@klkvr
fix: remove redundant check in bodies stage (paradigmxyz#12569)
56826cb
@emhane
chore(sdk): improve usability TxType trait (paradigmxyz#12548)
5f66fa4
@kamuik16
feat: implement Compact for OpTxType (paradigmxyz#12537)
ac5976f
@mattsse
feat: add helpers to obtain the execution witness for a payload (para…
841267d 
…digmxyz#12573)
@mattsse
feat: add payload witness fn (paradigmxyz#12579)
6e00e58
@klkvr @mattsse
feat: add tx_number consistency check to StaticFileProviderRW (paradi…
f0b8b9b 
…gmxyz#12570)

Co-authored-by: Matthias Seitz <[email protected]>
@mattsse @fgimenez
feat: add op debug witness api (paradigmxyz#12583)
b31b1ea 
Co-authored-by: Federico Gimenez <[email protected]>
@mattsse
feat: add contains fns to transport rpc modules (paradigmxyz#12593)
02237bf
@TheDhejavu
chore(util): Add reth payload util (paradigmxyz#12590)
5276093
@tcoratger
rm generics when useless (paradigmxyz#12581)
c160005
@Hopium21 @mattsse
Fix grammar in MDBX documentation (paradigmxyz#12580)
7745046 
Co-authored-by: Matthias Seitz <[email protected]>
@lakshya-sky
chore(sdk): add NetworkPrimitives for NetworkManager (paradigmxyz#12530)
2dc9a06 
Co-authored-by: dkathiriya <[email protected]>
@mattsse
chore: make clippy happy (paradigmxyz#12594)
fc97a0c
@emhane
chore(sdk): Remove duplicate trait BlockHeader (paradigmxyz#12584)
e182df7
@tcoratger
test(tx-pool): add more unit tests for parked pool (paradigmxyz#12591)
3614a37
@mattsse
chore: move hashing writer trait (paradigmxyz#12597)
1945cd8
@mattsse
chore: move triewriter trait (paradigmxyz#12598)
d52c719
@mattsse
chore: remove revm-primitives re-export (paradigmxyz#12599)
d9ed07a
@tcoratger
rm more generics when useless (paradigmxyz#12595)
dda1906
@klkvr @mattsse
chore(cli): unify trait bounds (paradigmxyz#12604)
735eb4b 
Co-authored-by: Matthias Seitz <[email protected]>
@fgimenez
feat: split reth-payload-primitives (paradigmxyz#12600)
bf92a5f
@fgimenez
feat: fix WASM build for crates dependent on reth-payload-primitives (p…
2f3fde8 
…aradigmxyz#12610)
@emhane
chore(sdk): Add blanket impls for refs to prim traits (paradigmxyz#12613
7ae8ce1 
)
mattsse and others added 30 commits November 26, 2024 16:21
@mattsse
chore: unify more tx signed fns (paradigmxyz#12883)
ebf837e
@ftupas
feat: add utility trait methods to Transaction (paradigmxyz#12704)
d51b347
@klkvr
feat: relax provider bounds (paradigmxyz#12857)
2ce7417
@mattsse
chore: make cargo t compile for reth-trie-common (paradigmxyz#12884)
5dc914b
@frisitano @mattsse
Introduce StateCommitment in StateProviders (paradigmxyz#12602)
83af493 
Co-authored-by: Matthias Seitz <[email protected]>
@klkvr
refactor: improve state writing functions for db provider (paradigmxy…
38cf6c9 
…z#12885)
@mattsse
chore: fix unused warning (paradigmxyz#12888)
e8d63e4
@mattsse
feat: introduce networkprimitives in transition fetcher (paradigmxyz#…
02f3427 
…12889)
@hai-rise
fix(ci): warn instead of failing no tests (paradigmxyz#12893)
6b3c7c6
@hai-rise
perf: minimize clones when saving blocks (paradigmxyz#12870)
acfcfbd
@PelleKrab
added generic header (paradigmxyz#12897)
3b8c661 
Co-authored-by: router <[email protected]>
@zitup
chore(sdk): Add MaybeArbitrary to all ATs on NodePrimitives (paradigm…
51afa4c 
…xyz#12847)
@mattsse
feat: extends engine validator (paradigmxyz#12900)
b33757f
@programskillforverification @emhane
chore: make BeaconConsensusEngineEvent generic over data primitives (p…
a0e2961 
…aradigmxyz#12831)

Co-authored-by: Emilia Hane <[email protected]>
@klkvr
refactor: unify code paths for receipts removal (paradigmxyz#12887)
42c24f0
@emhane
Upcast trait bound on BlockBody::Transaction to SignedTransaction (…
db9b86a 
…paradigmxyz#12903)
@mattsse
feat: add engine validator addon (paradigmxyz#12905)
b62929c
@klkvr
feat: add Receipt AT to ReceiptProvider (paradigmxyz#12890)
533b555
@fgimenez @rkrasiuk @shekhirin
feat(engine): proof fetching on state update for StateRootTask (parad…
1131bde 
…igmxyz#12458)

Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: Alexey Shekhirin <[email protected]>
@klkvr
feat: add Receipt AT to writer traits (paradigmxyz#12892)
7a6a725
@klkvr
fix: small db provider fixes (paradigmxyz#12908)
7a60530
@rkrasiuk
feat: SparseStateTrie::reveal_multiproof (paradigmxyz#12909)
26bfe7c
@mattsse
chore: add fs-util::open (paradigmxyz#12911)
f9ad3f8
@mattsse
feat: add helpers for opcode tracing (paradigmxyz#12899)
2705e3a
@rkrasiuk
feat(trie): SparseStateTrie::remove_storage_leaf (paradigmxyz#12912)
2700db1
@mattsse
feat: integrate node primitives in engine handler (paradigmxyz#12914)
00c5b69
@rkrasiuk
feat(trie): SparseStateTrie::storage_trie_mut (paradigmxyz#12913)
4724564
@htiennv
chore: pass generic header to validate_header_base_fee (paradigmxyz#1…
8d70e89 
…2921)
@mattsse
feat: add functions for env creation (paradigmxyz#12928)
2179301
@0xObsidian
fix: addressed lockfile and tempdir issues
60b0f98 
Two issues were causing static file test failures:
1. Missing lockfile in test file count
2. Premature cleanup of temporary test directories

Bug Description
===============
1. File Count Mismatch:
   - Tests expected 10 files (9 static files + 1 lockfile)
   - Only had 9 files because lockfile wasn't created
   - Led to assertion failures in file count checks

2. Directory Cleanup Race:
   - TempDir was being dropped too early due to pattern matching:
     ```
     let (static_dir, _) = create_test_static_files_dir();
     ```
   - Caused test directory to be cleaned up while tests were still running
   - Worked in isolation but failed in full test suite

Reproduction Steps
==================
Pull the previous commit and run from project root:
```
make test-unit
```

Fix
===
- Added lockfile creation before static file operations
- Fixed file count assertions
- Fixed premature cleanup of test directories

Testing the fix
===============
Pull ths commit and run the test suite:
```
make test-unit
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.