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

Add good full coverage beacon API tests #3682

Closed
dapplion opened this issue Jan 29, 2022 · 2 comments
Closed

Add good full coverage beacon API tests #3682

dapplion opened this issue Jan 29, 2022 · 2 comments
Labels
meta-technical-debt Issues introducing or resolving technical debts. prio-medium Resolve this some time soon (tm). scope-testing Issues for adding test coverage, fixing existing tests or testing strategies.

Comments

@dapplion
Copy link
Contributor

What is your question?

Current tests for the beacon API are of sub-optimal quality:

  • Very limited coverage
  • No assurance that all and future new endpoints are tested
  • Require import stubbing
  • Extensive use of brittle stubs: which require big diffs when modifying unrelated cost.

In my subjective opinion the current test suite has a very high cost of maintenance with very little return in ensuring the API is correct.

Proposed solution

Proper tests for a the beacon API should:

  • Have full coverage
  • Ensure programmatically that all endpoints current and future are tested
  • Be tested against a full BeaconChain instance without stubbing and with real data. We are already doing this for BeaconChain validation and such data is extremely fast to produce and to run tests against.
@philknows philknows added meta-technical-debt Issues introducing or resolving technical debts. prio-high Resolve issues as soon as possible. labels Mar 18, 2022
@dapplion dapplion added the scope-testing Issues for adding test coverage, fixing existing tests or testing strategies. label May 20, 2022
@dapplion
Copy link
Contributor Author

dapplion commented May 20, 2022

More disabled tests

// TODO remove stub
describe.skip("beacon api impl - state - get fork", function () {

// TODO remove stub
describe.skip("beacon pool api impl", function () {

// TODO remove stub
describe.skip("api - debug - beacon", function () {

// TODO remove stub
describe.skip("block assembly", function () {

This was referenced May 25, 2022
@dadepo dadepo self-assigned this May 26, 2022
@philknows philknows added this to the v1.0.0 pre-merge milestone Jun 10, 2022
@philknows philknows added prio-medium Resolve this some time soon (tm). and removed prio-high Resolve issues as soon as possible. labels Jun 29, 2022
@philknows philknows removed this from the v1.0.0 pre-merge milestone Oct 10, 2024
@wemeetagain
Copy link
Member

We now have oapi schema compliance tests that ensure validity of request/response data shapes and proper support of headers, status codes, etc. This is a good halfway solution.
Closing this for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta-technical-debt Issues introducing or resolving technical debts. prio-medium Resolve this some time soon (tm). scope-testing Issues for adding test coverage, fixing existing tests or testing strategies.
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants