chore: convert ava tests to mocha #975
Conversation
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@typescript-eslint/[email protected], npm/[email protected] |
There was a problem hiding this comment.
Thanks for taking this on! Glad you were able to keep these pretty similar while dropping the ava dependency.
But that actually makes me wonder how @Shaptic feels about them. I think it's a little bit silly that we have the tests/e2e directory at all, or a separate script in scripts to run them. Are these really end to end tests? Maybe we could just name them something more accurate and I'd be happy with them.
Which kind of leads to larger questions.
- Do we like the other directories, inside
test?unitseems to contain a lot of tests that aren't, strictly-speaking, unit tests- this new
e2estuff is sort of integration-type tests, but theintegrationdirectory doesn't look similar enough to make sense to re-use this directory.
- only one file within
unitauthors the test itself in TypeScript. Should all the tests be authored in TypeScript? (Note that this will lead to some (surmountable) typing challenges with thesee2etests, sinceAssembledTransactionis generic and would need to be explicitly typed (or explicitly ignored) in the TS files.)
Maybe we don't have to answer all of these questions now. However we want to evolve this codebase, having fewer dependencies and testing tools is a good thing!
Aside from those higher-level questions, I noticed some small things we can improve below.
There was a problem hiding this comment.
Wow you rock for powering through on this 👏
@chadoh I definitely agree that the testing is a little messy and I don't have great answers for your questions at the moment (especially e2e since I don't even know how those work, strictly speaking), but this PR is definitely a great move in the right direction in terms of cleaning them all up 👏
BlaineHeffron
force-pushed
the
chore/convert-tests
branch
from
f7b853d
to
8bed9f4
Compare
BlaineHeffron
force-pushed
the
chore/convert-tests
branch
from
8bed9f4
to
793aab3
Compare
Signed-off-by: blaineheffron <[email protected]>
remove line Co-authored-by: Chad Ostrowski <[email protected]> Signed-off-by: blaineheffron <[email protected]>
Signed-off-by: blaineheffron <[email protected]>
Signed-off-by: blaineheffron <[email protected]>
Signed-off-by: blaineheffron <[email protected]>
more precise wording Co-authored-by: Chad Ostrowski <[email protected]> Signed-off-by: blaineheffron <[email protected]>
Signed-off-by: blaineheffron <[email protected]>
Signed-off-by: blaineheffron <[email protected]>
clean up nested awaits Co-authored-by: Chad Ostrowski <[email protected]> Signed-off-by: blaineheffron <[email protected]>
BlaineHeffron
force-pushed
the
chore/convert-tests
branch
from
793aab3
to
79aaf19
Compare
Converts the e2e tests to using mocha. Removes the ava dependency.