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

docs(fake-timers): added a page for using MikroORM with Jest, with fake timers info #5157

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

boenrobot
Copy link
Collaborator

@boenrobot boenrobot commented Jan 19, 2024

Closes #4994.


I know what I said before about glob on init, but... I tried to locate the exact source, and couldn't find it now... I'm guessing maybe they changed in some recent versions, and maybe I was looking at an outdated one at the time. Anyhow, I haven't included it in the docs due to that.

The code shown for a workaround is a more polished version of what I showed in #4994, and after some research, I found a workaround for Mongo that I also included.


EDIT: Workaround provided is probably a bit too hacky... I'm researching something more elegant and less memory leak-y.


EDIT2: OK, I think I have a better workarounds for each driver now, targeting specifically the things that absolutely need a real process.nextTick() and nothing else.

Copy link

codecov bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ff4dc61) 99.75% compared to head (981ba24) 99.75%.
Report is 26 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5157   +/-   ##
=======================================
  Coverage   99.75%   99.75%           
=======================================
  Files         221      221           
  Lines       16571    16585   +14     
  Branches     4016     4020    +4     
=======================================
+ Hits        16530    16544   +14     
  Misses         41       41           

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

@B4nan
Copy link
Member

B4nan commented Jan 30, 2024

It looks great overall. A few notes for now:

@boenrobot
Copy link
Collaborator Author

boenrobot commented Jan 30, 2024

  • I dont like that this is "jest specific section", isnt the same problem in vitest (and possibly other test frameworks) nowadays too?

Not necessarily... In the case of vitest, according to their docs, they don't mock process.nextTick() by default, unless you opt in (as opposed to Jest, where you opt out)... except if you do opt in, whether it works depends... Seems like someone using vitest's fake timers would be more aware of potential process.nextTick() hangs due to all of those docs there. Not to mention they're less likely to hit that issue due to it not being mocked by default.

Fixed

the code is a bit too complex for a docs page, have you considered publishing that as your own package? :]

Realistically, it would end up as abandonware if I was to publish it, because I am not using any testing framework for my employer's projects... yet. Not to mention that for most people, not mocking nextTick is a perfectly fine workaround.

This has a better chance to be kept up to date if someone more keen personally invested would take it on, and better likelyhood to be discovered and actually used if it's part of this monorepo or at least under this organization.

lets have a general "Testing" page instead, could be a recipe, but maybe better move it to some more prominent section too? we should also mention how to setup vitest there (thats covered in the getting started guide only nowadays)

I'm not sure what to include in such a section that wouldn't already be in the getting started guide of the test framework or the initial setup of MikroORM already.

@B4nan
Copy link
Member

B4nan commented Jan 30, 2024

Not necessarily...

Even if it's not the case, I still want one testing page in docs, not multiple recipes for different frameworks, as each has its own quirks and I want to have this documented at one place (later on, when we have more content, we can have more pages in a dedicated section).

I'm not sure what to include in such a section that wouldn't already be in the getting started guide of the test framework or the initial setup of MikroORM already.

Vitest requires additional setup for folder-based discovery to work, that's one thing, and later on even more (e.g. mocking). Getting started guide is not supposed to be the single source of truth, it already duplicates most of the information that is in there.

And there are more things we should add there going forward, like the required metadata discovery for working with entity classes (so propagation can work). People often ask for some suggestions around testing, we need to have a place to put them into.

This has a better chance to be kept up to date if someone more keen would take it on,

Well, by merging it into master, you are making it my responsibility. I get what you mean, but realistically, if you as its author won't be the one who maintains it, it will likely get outdated in the long run anyway. But let's not discuss that further, I am fine merging it in, it's just weird to have that in the docs as it's pretty complex code on its own.

and better likelyhood to be discovered and actually used if it's part of this monorepo or at least under this organization.

That's not really an argument, the package would be linked and used in the examples in the docs here, making it discoverable the same way. I'd argue it's the opposite, if you turn it into a package, its no longer "some piece of code in MikroORM docs" but something universal people can use with other ORMs too - as the problem is not directly connected to the ORM but to its dependencies.

@B4nan B4nan merged commit dc05ac1 into mikro-orm:master Feb 3, 2024
11 checks passed
@B4nan
Copy link
Member

B4nan commented Feb 3, 2024

I'll move things around once we have the dedicated testing section, let's move this forward as is.

Thanks again!

@boenrobot boenrobot deleted the fakeTimerDocs branch February 10, 2024 14:25
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.

docs: convey that jest.useFakeTimers() can break MikroORM without specifying doNotFake: ["nextTick"]
2 participants