-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Foundry prbmath external test #13873
Conversation
349c8b9
to
b512bd7
Compare
eda4a30
to
55464fd
Compare
7ba7c7e
to
47b4ec0
Compare
47b4ec0
to
ea839c6
Compare
3486a83
to
5468baf
Compare
f37184a
to
39aac12
Compare
26fe47f
to
f1a27c2
Compare
39aac12
to
d77f8ad
Compare
414d669
to
6b24116
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's more feedback.
I also pushed some new commits that address minor things that weren't worth starting a discussion.
98578ff
to
1605683
Compare
3d46980
to
300f32f
Compare
e01b456
to
f777a98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm approving since this is already good enough.
There are still a few minor comments that I see you're in process of addressing and you need to squash, but I'm assuming all of that will be dealt with. Feel free to merge once you're done.
006df04
to
469dd8b
Compare
469dd8b
to
0940a56
Compare
0940a56
to
b802782
Compare
…in python Co-authored-by: Kamil Śliwak <[email protected]>
b802782
to
dcbd645
Compare
Fixes #13767.
This PR migrates the
prb-math
external tests script to python and add support to foundry.The PR also adds an initial architecture to the python-based external tests. The idea was to allow us to easily migrate from different test frameworks as external projects change their dependencies.
I didn't migrate all the shellscripts yet, for instance, the
store_benchmark_report
still missing for theprb-math
. But it would be nice to discuss some ideas around the current architecture.Depends on: #13982, #14064 and #14140.
The dependence on Cleanup base images and CI config #13982 is due to the change of the t_ems_ext, which was renamed to
t_ext
and now receives theimage
as parameter instead of only the nodejs version. This allows running some external tests on different base images, e.g.cimg/rust:1.66.1
.The change was required since Foundry depends on rust and not nodejs. Actually, as we are now downloading theforge
binary, rust is not needed.The dependence on External tests script refactor #14064 is based on the replacement of the old
test/externalTests.sh
by a python script that allows run one, multiple or all external tests.The dependence on Foundry ci setup #14140 is due to the new
install_python3
andinstall_foundry
CI commands.TODO:
parse_eth_gas_report.py
(will be done in a future PR)