sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
802 stars 105 forks source link

What does this sentence in the readme actually mean ("...use your imagination ...") #343

Closed tchakabam closed 3 years ago

tchakabam commented 3 years ago

I consider this a documentation bug:

If you want to simulate asynchronous behavior, you have to use your imagination when calling the various functions.

This is ambiguous (and seems wrong to me) in a lot of ways. Please explain what exactly this is supposed to mean.

fatso83 commented 3 years ago

I agree this is unclear. I am also not sure what is meant by this, TBH, but it might mean that a simulation in essence is a made-up scenario, which of course is something you have to make up. That might be what the author meant, or less likely, it could be that he was thinking of how to stub the behavior at a technical level.

tchakabam commented 3 years ago

I understand now also that there are specific functions to simulate async behavior in the API.

I guess that would be a better type of instruction to give, say: "Use the intended for methods below regarding async processes"

fatso83 commented 3 years ago

I guess we will never know the true intent, as @cjohansen wrote that in 2013, but you are free to come up with a PR to make things better :) Closing for now.

cjohansen commented 3 years ago

The context for this piece of unhelpful text was that you have a synchronous test and want to test an asynchronous flow using stubbed functions. There's no good way to do this in a general way, so the docs suggested you got imaginative with how you ordered function calls and assertions. Not saying it's a good piece of documentation, but that probably was the intent of it 🙃

tchakabam commented 3 years ago

@cjohansen I don't think that makes much sense, any test can be made async, with all test runners I know.

cjohansen commented 3 years ago

Dude, I provided an explanation for some text that was written 10 years ago. What's the point of arguing the reasoning in light of what's possible today? 🙄

tchakabam commented 3 years ago

My name is Stephan :) Hi Christian.

I think that it was definitely possible 10 years ago (and even since JS very start) to do:

myTest(doneCallback) {
  setTimeout(function() {
    // do whatever async
   doneCallback()
  }, 0);
}

Right?

As JS is a language that in its concept runs atop an event-loop to allow manage async proceses in the background and scheduling/dispatching events in this way.

It is just that, I like it when talking about technical things to be precise and correct. I don't think anyone is helped with things that don't really make sense.

If you post something in a forum that Github is, then you have to also allow/expect that people challenge that with logic and reason I think. In the end, everyone should benefit from that. Even better, when people use a pleasant tone and pleasing language quality when doing so.

Cheers!

cjohansen commented 3 years ago

I think that it was definitely possible 10 years ago (and even since JS very start) to do:

This was not possible with JsTestDriver at the time this was written.

As JS is a language that in its concept runs atop an event-loop to allow manage async proceses in the background and scheduling/dispatching events in this way.

I can barely understand this incredibly condescending piece of writing, but you can trust that the authors of this library - present and past - understand the language. Assuming they don't is arrogant and not very helpful.

It is just that, I like it when talking about technical things to be precise and correct. I don't think anyone is helped with things that don't really make sense.

I agree, which makes it all the more of a mystery why you are still adding to this thread.

If you post something in a forum that Github is, then you have to also allow/expect that people challenge that with logic and reason I think. In the end, everyone should benefit from that. Even better, when people use a pleasant tone and pleasing language quality when doing so.

I'll take challenges any day from anyone with basic decency and the necessary context to do so. You started off with an entitled message: "this is a documentation bug" - ie. "your free piece of software is not up to my standards, please fix it". Even so, you were met with a friendly and welcoming tone. Still no PR from you, though.

I thought it would be fun to provide a little bit of context on why the text was written in the first place, and now I have take condescending "corrections" (which are still wrong) from you. Thanks to bullshit like this, I will not chip in on matters on this repo again.

mantoni commented 3 years ago

Maybe I can help resolving the misunderstanding here.

It's obviously possible to run async code in JS, and That was always the case. We can all agree on that. It's also clear that test can be written that test this async behavior.

However, with the Sinon fake timers, it's also possible to program the clock and cause timer invocations to be triggered by the test synchronously. When developers opt into doing this, they make assumptions about timings and how the Event queue works. While this is trivial in most cases, it's easily possible to make wrong assumptions and cause code execution orders in tests that aren't possible in "real life".

Christians comment was referring to this fact, maybe in a not very clear way, back in the day, when Sinon was a young project, used by just a few. We're aiming to have better documentation, now that Sinon matured and is used by a broad user base.

We're just a handful of developers who do this in their spare time. I'd like to invite you to contribute to the documentation wherever you feel like things could be expressed more clearly.

❤️

benjamingr commented 3 years ago

As JS is a language that in its concept runs atop an event-loop to allow manage async proceses in the background and scheduling/dispatching events in this way.

It is just that, I like it when talking about technical things to be precise and correct. I don't think anyone is helped with things that don't really make sense.

I hate to be that pedantic person but until ES2015 it was impossible to write async software in JavaScript.

The async bit came from the host environment. Even in ES2015 the async part is only "run to completion" microtask semantics and "real" asynchronicity comes from the platform.

You can plainly see this by looking at the fake-timers source code and seeing that there are a bunch of if/else depending on what platform we run on (since Node implements its own timers and the web implements the web timers spec). You can also trust me as both a maintainer here and a Node.js member (and also a WHATWG one, but I haven't actually contributed to the timers spec) - but I am happy to provide references to any/all this.


Regardless of the technical/pedantic bit - please assume good faith (and competence) from maintainers of free libraries you use :)

You are of course welcome to contribute docs back (please do) or (try and) pay Christian (or other people) to improve them.

(As a side note, we have a lot of "leaves a lot to be desired" docs in all other projects I'm a part of)

niekcandaele commented 2 years ago

Hi everyone

Sorry for necro posting. I stumbled on this issue while trying to use fake-timers for some async stuff myself and had missed that there were async versions of the functions.

I made a small PR to adjust that one problematic sentence in the README, hopefully this will help other people find a solution quicker in the future.


As an outsider to this repo, I'd like to say that I was absolutely appalled by the comments here. It took me 5 minutes to make this PR and (IMO) make fake-timers better than it was before. I bet it took longer to write these condescending/entitled/rude comments. Just a matter of how you like to spend your time I suppose :)

@ maintainers please keep doing what you are doing, fake-timers is massively useful, even after 7 years. Thank you!