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

run tests against future forks by default #2635

Merged
merged 1 commit into from
Oct 2, 2021
Merged

run tests against future forks by default #2635

merged 1 commit into from
Oct 2, 2021

Conversation

etan-status
Copy link
Contributor

Some tests are currently restricted to a single phase using @with_phases
even though they could likely run unchanged in later phases. This patch
changes the default for such tests to also run in later phases. If the
beacon chain changes enough in later phases to break these tests, this
highlights that the tests need to be adjusted or extended accordingly.

@etan-status etan-status marked this pull request as ready for review September 28, 2021 21:34
@@ -45,7 +48,7 @@ def wrap(flag: AtomicBoolean):
assert is_called.value


@with_phases([MERGE])
@with_merge_and_later
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought test_on_merge_block.py tests are MERGE-phase-only?
/cc @zilm13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH, I'm not 100% sure on these, I'll revert the test_on_merge_block.py changes for now until there is clarity.

Some tests are currently restricted to a single phase using @with_phases
even though they could likely run unchanged in later phases. This patch
changes the default for such tests to also run in later phases. If the
beacon chain changes enough in later phases to break these tests, this
highlights that the tests need to be adjusted or extended accordingly.
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

lgtm, thank you!

@hwwhww hwwhww merged commit cc0bb7a into ethereum:dev Oct 2, 2021
@etan-status etan-status deleted the with-altair-later branch October 4, 2021 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants