Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Add basic API tests for re-exports #234

Closed
holgerd77 opened this issue Feb 4, 2020 · 0 comments · Fixed by #235
Closed

Add basic API tests for re-exports #234

holgerd77 opened this issue Feb 4, 2020 · 0 comments · Fixed by #235

Comments

@holgerd77
Copy link
Member

Along with an update of the version of the secp2561 dependency #228 we were close to updating a re-export with a breaking version within a minor ethereumjs-util release, see the comment from @alcuadrado for context.

To have this more structurally addressed and not only rely on reviewer awareness we should add some test cases running some API functionality of the re-exports (BN.js, RLP, secp2561). I would suggest to start slow here and add 3-5 test cases of central functionality of the libraries (we can't realistically test the whole API) for each re-export to the new test/externals.spec.ts test file.

This won't be the everything-covered solution but already significantly enlarge the chance to catch structural changes like a reworked error handling or a Node version dropped in the re-export library.

I would suggest (first thought, open for improvements along the way) to do at least the following tests for each library:

  • Use a function as intended
  • Trigger a well-defined error case on a function
  • Eventually trigger a not-so-well defined error case (wrong input)

The error case differentiation is a bit blurry, might be best to decide on a case-by-case basis what is practical.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants