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

Turn off light client tests for feature specs #3389

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Turn off light client tests for feature specs #3389

merged 2 commits into from
Jun 1, 2023

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented May 25, 2023

Issue

The current test suite requires light client specs by default, even though most feature specs do not focus on light client design.

IMO unless a feature specifically pertains to light client design, these light client specs within feature specs serve little purpose.

How did I fix it

  • Create a LIGHT_CLIENT_TESTING_FORKS setting and set it from Altair to Deneb.
  • Use with_light_client decorator for the light client tests
  • Delete EIP-6110 light client specs

It will help minimize the overhead associated with adding new feature specs.

Copy link
Collaborator

@mkalinin mkalinin left a comment

Choose a reason for hiding this comment

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

I am pretty much in favour of this proposal! It allows to get rid of unnecessary complexity required to define a new feature

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

I support this!

light client spces should probably be built for a particular Fork or as a particular feature itself (EIP). Otherwise, it's just a ton of typing and boilerplate work for no gain

@hwwhww hwwhww merged commit 0d4b07f into dev Jun 1, 2023
@hwwhww hwwhww deleted the fork-settings branch June 1, 2023 06:59
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