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

Validate geth version in test fixture against binary used for tests #3193

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Jan 18, 2024

What was wrong?

We've had instances where we forget to update the geth version being used in the circleCI runs and this goes unnoticed since two versions can be compatible with each other. This means we create the fixture with one version, but run it with another - and, as long as no noticeable changes have happened, this is silently unintended.

How was it fixed?

  • Validate the geth binary version being used to run the tests against the version in the generated zip file for the test fixture. This version is automatically named in the zip file to be the geth version used to create the zip so it should be a decent enough check.

Todo:

Cute Animal Picture

20240118_112335

@fselmo fselmo force-pushed the check-geth-fixture-version-with-binary-used-for-tests branch 2 times, most recently from 29dd16f to fd177c8 Compare January 18, 2024 18:26
fselmo added a commit to fselmo/web3.py that referenced this pull request Jan 18, 2024
@fselmo fselmo force-pushed the check-geth-fixture-version-with-binary-used-for-tests branch from 1e14e72 to 4aaca65 Compare January 18, 2024 18:29
@fselmo fselmo marked this pull request as ready for review January 18, 2024 18:29
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

As a follow up, we might consider updating the version as part of the fixture generation script so we don't have to remember in the future.

Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fselmo fselmo merged commit c4f3a8c into ethereum:main Jan 19, 2024
99 checks passed
@fselmo fselmo deleted the check-geth-fixture-version-with-binary-used-for-tests branch April 3, 2024 20:50
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.

None yet

3 participants