Skip to content

Turn off light client tests for feature specs#3389

Merged
hwwhww merged 2 commits intodevfrom
fork-settings
Jun 1, 2023
Merged

Turn off light client tests for feature specs#3389
hwwhww merged 2 commits intodevfrom
fork-settings

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.

@hwwhww hwwhww added lightclients testing CI, actions, tests, testing infra labels May 25, 2023
@hwwhww hwwhww requested a review from djrtwo May 25, 2023 09:34
Copy link
Contributor

@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

Labels

lightclients testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants