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

tests: Improves testing for all the fork logic so that the tests never fail #597

Merged
merged 6 commits into from
Oct 10, 2023

Conversation

0xJepsen
Copy link
Collaborator

@0xJepsen 0xJepsen commented Oct 9, 2023

What I did:

  • Used rayon to run a bunch of iterations of the tests concurrently
  • Cleaned up files after every iteration so that every PR no longer has a dif if tests were run.
  • Used assertions on results to check is_ok()

I also got roasted by chat gpt when i was asking it if i could grab any functionality from the binary into the lib. said that if this is the case it's an indication i should move the logic from the bin to the lib. I think this is correct long term but for now i just moved the fork_into_arbiter test into the bin tests since it was able to get everything we needed. Did some small cleanup around this to make it easier more modular for the future.

This PR also changes CI to run on every PR now, not just ones into main.

@0xJepsen 0xJepsen requested review from Autoparallel, kinrezC and Alexangelj and removed request for Autoparallel October 9, 2023 18:56
debug line
revert debug changes
remove comment
fmt
move weth

runs CI on all prs now

comments

tests pass

move test to bin

fix fork test
@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #597 (63b01de) into arbiter/version (1ee29f9) will decrease coverage by 2.87%.
The diff coverage is 87.30%.

❗ Current head 63b01de differs from pull request most recent head 167861b. Consider uploading reports for the commit 167861b to get more accurate results

@@                 Coverage Diff                 @@
##           arbiter/version     #597      +/-   ##
===================================================
- Coverage            63.56%   60.69%   -2.87%     
===================================================
  Files                   25       26       +1     
  Lines                 4394     5427    +1033     
===================================================
+ Hits                  2793     3294     +501     
- Misses                1601     2133     +532     
Files Coverage Δ
arbiter-core/src/bindings/weth.rs 46.49% <ø> (ø)
bin/fork/tests.rs 95.52% <94.44%> (-4.48%) ⬇️
bin/fork/mod.rs 88.34% <44.44%> (-2.66%) ⬇️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@0xJepsen 0xJepsen linked an issue Oct 9, 2023 that may be closed by this pull request
@0xJepsen 0xJepsen merged commit 6518d59 into arbiter/version Oct 10, 2023
4 of 5 checks passed
@0xJepsen 0xJepsen deleted the testing branch October 10, 2023 17:40
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.

Fork Test Fail on occasion
1 participant