dcramer / peated

https://peated.com
Apache License 2.0
59 stars 12 forks source link

Collapse worker into server and various refactor #153

Closed dcramer closed 3 months ago

dcramer commented 3 months ago
dcramer commented 3 months ago

Two core issues:

  1. the obscure error happening happening (bottom of this image):

image

  1. mock is broken and had to be inlined temp
dcramer commented 3 months ago

opened https://github.com/chaijs/check-error/issues/51 as part of this, but ultimately whats happening (afaik) is something changed, possibly with vitest, where the expect'd promise rejection tests are now trigger an uncaught promise rejection, and the error was being masked

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 81.05263% with 126 lines in your changes are missing coverage. Please review.

Project coverage is 78.29%. Comparing base (0caeb6f) to head (7b02f75).

:white_check_mark: All tests successful. No failed tests found :relaxed:

Files Patch % Lines
apps/server/src/trpc/routes/bottleCreate.ts 83.23% 28 Missing :warning:
apps/server/src/lib/test/fixtures.ts 91.52% 20 Missing :warning:
apps/server/src/lib/scraper.ts 71.18% 17 Missing :warning:
apps/server/src/worker.ts 0.00% 16 Missing :warning:
apps/server/src/lib/test/waitError.ts 83.33% 9 Missing :warning:
apps/server/src/jobs/scrapeAstorWines.ts 57.14% 6 Missing :warning:
apps/server/src/jobs/scrapeHealthySpirits.ts 57.14% 6 Missing :warning:
apps/server/src/jobs/scrapeTotalWine.ts 57.14% 6 Missing :warning:
apps/server/src/jobs/scrapeWoodenCork.ts 57.14% 6 Missing :warning:
apps/server/src/jobs/generateBottleDetails.ts 0.00% 2 Missing :warning:
... and 9 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #153 +/- ## ========================================== - Coverage 81.58% 78.29% -3.29% ========================================== Files 179 187 +8 Lines 12466 13737 +1271 Branches 1064 1140 +76 ========================================== + Hits 10170 10755 +585 - Misses 2296 2982 +686 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dcramer commented 3 months ago

I've rewritten the promise rejection tests to use trpc's waitError pattern (which should avoid the dangling promise concern, hopefully). Unfortunately the goal of also using inline snapshots has led to yet a new random error

image

dcramer commented 3 months ago

For future me:

All of the issues were caused by vitest and vitest-mock-axios.

The vitest-mock-axios package was installing a different version of vitest, which somehow conflicts with itself (I'm guessing it patches/does codegen), which was causing the rejected promises error as well as the snapshot error.

Replacing that with mock-axios-adapter and everything works as expected.

More context here https://github.com/chaijs/check-error/issues/51

dcramer commented 3 months ago

🙏