-
Notifications
You must be signed in to change notification settings - Fork 232
Refactor tests to reduce duplication for different renderers #643
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
Conversation
Codecov Report
Merging #643 (49cf925) into main (eff2ca6) will not change coverage.
The diff coverage isn/a.
@@ Coverage Diff @@ ## main #643 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 15 15 Lines 235 235 Branches 29 33 +4 ========================================= Hits 235 235
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update eff2ca6...49cf925. Read the comment docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't mind having another go at this test file sometime (not in this change). Currently, it the longest running test by far, clocking almost 20s on my macbook to run all the tests for all the renderers. We had problems in the past making the delays too small, but that was before the refactoring of the asyncUtils which I think has made things more reliable.
993d2c5 to
49cf925
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great to see us loose so much duplication code! Also this way for managing the test suites is also super interesting to see 🚀
What:
Refactor tests to reduce duplication for different renderers.
Why:
Our test suite has lots of duplication, running the same (or almost the same) test against the
dom,nativeandserverrenderers.How:
Added a test utility that runs tests against groups of renderers.
Checklist:
For some reason, the current tests were not having
eslintwarnings appear. When moving them intosrc/__tests__/, the warnings appeared. This actually highlighted an issue with the type of thecleanupexport which is is anasyncfunction, but wasn't returning aPromisein the types. I've included updating them in this PR so we must remember to merge this as afixrather that thetestcommit.