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

Collapse worker into server and various refactor #153

Merged
merged 41 commits into from
Apr 9, 2024
Merged

Conversation

dcramer
Copy link
Owner

@dcramer dcramer commented Apr 6, 2024

  • Move @peated/worker into @peated/server
  • Add defaults and fixtures to default fixture injection for vitest
  • Various dep updates
  • Cleanup fixtures to run primarily in transactions
  • Restore sequence reset in tests

@dcramer
Copy link
Owner Author

dcramer commented Apr 6, 2024

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
Copy link
Owner Author

dcramer commented Apr 7, 2024

opened chaijs/check-error#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

Copy link

codecov bot commented Apr 8, 2024

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).

✅ All tests successful. No failed tests found ☺️

Files Patch % Lines
apps/server/src/trpc/routes/bottleCreate.ts 83.23% 28 Missing ⚠️
apps/server/src/lib/test/fixtures.ts 91.52% 20 Missing ⚠️
apps/server/src/lib/scraper.ts 71.18% 17 Missing ⚠️
apps/server/src/worker.ts 0.00% 16 Missing ⚠️
apps/server/src/lib/test/waitError.ts 83.33% 9 Missing ⚠️
apps/server/src/jobs/scrapeAstorWines.ts 57.14% 6 Missing ⚠️
apps/server/src/jobs/scrapeHealthySpirits.ts 57.14% 6 Missing ⚠️
apps/server/src/jobs/scrapeTotalWine.ts 57.14% 6 Missing ⚠️
apps/server/src/jobs/scrapeWoodenCork.ts 57.14% 6 Missing ⚠️
apps/server/src/jobs/generateBottleDetails.ts 0.00% 2 Missing ⚠️
... and 9 more
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dcramer
Copy link
Owner Author

dcramer commented Apr 8, 2024

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 dcramer force-pushed the ref/collapse-worker branch from caa7ae5 to 0b6ba20 Compare April 8, 2024 20:06
@dcramer
Copy link
Owner Author

dcramer commented Apr 9, 2024

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 chaijs/check-error#51

@dcramer
Copy link
Owner Author

dcramer commented Apr 9, 2024

🙏

@dcramer dcramer merged commit b22fa33 into main Apr 9, 2024
3 of 5 checks passed
@dcramer dcramer deleted the ref/collapse-worker branch April 9, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant