mochajs / mocha

☕️ simple, flexible, fun javascript test framework for node.js & the browser
https://mochajs.org
MIT License
22.63k stars 3.02k forks source link

🚀 Feature: Support generator functions for tests #1932

Closed antpaw closed 10 months ago

antpaw commented 9 years ago

there are npm modules that make mocha work with generator functions (like this one https://github.com/vdemedes/mocha-generators) But mocha should at least fail the test, because this is horrible behaviour in production

  it('meh', function*() {
    expect(0).to.equal(1);
  });
// this will pass :(
ajsharp commented 8 years ago

Worth noting that the same thing happens when using generators with bluebird's coroutine wrapper.

ScottFreeCode commented 8 years ago

Why would you pass Mocha a generator function instead of a regular one anyway? It makes about as much sense as passing Mocha a constructor, and has roughly the same problem -- calling the function doesn't do stuff, it just returns an object that can be used to do other stuff. It's as if you wanted to have special handling to protect yourself from doing this:

it("is basically just like a generator function without yield", function() {
  return {
    next: function() {
      expect(0).to.be(1);
    }
  };
});
antpaw commented 8 years ago

because i have integration tests running with generators, and if i forget to include co-mocha all test pass. https://github.com/segmentio/nightmare (see the part about mocha in the readme.md)

ScottFreeCode commented 8 years ago

@boneskull, @dasilvacontin, any opinions? I am not familiar enough with generators to know whether the test in that readme's example couldn't be done with promises and/or done.

ajsharp commented 8 years ago

@ScottFreeCode The desire to use generator functions is to be able to use yield and other such functionality that comes along with a typical use of generators. Though maybe off-topic, what really would be great is if mocha supported using generator functions in a similar way to bluebird, co or other such libraries "wrap" a generator function.

ScottFreeCode commented 8 years ago

@ajsharp Can you elaborate on what functionality that is and how it would be used in a test? I've read some articles on generator functions and yield, but somehow all they told me is that the generator function is basically a constructor for another function object that uses yield, and that yield is basically a return statement (in the sense of outputting a value) that can be used multiple times (each subsequent yielded value being returned from respective calls to the function produced by the generator). Given that a test function only runs once, and that the only use of the return value is promises (which already work great for testing asynchronous code via their chaining), I still am not clear on what yielding in the test function itself would accomplish.

Tangentially, what if you call the generator function itself (to get the function it returns that actually runs with yield) before passing its output to Mocha, rather than passing the whole generator to Mocha? E.g.

it("should do something magical with next-generation JavaScript", function*() {
  // insert whatever yield is supposed to do here
}()); // instead of });

That would give Mocha the actual test function to run without Mocha needing to handle generators specially; but, again, from what info I found last time I read up on them I don't understand how yield and such are supposed to change tests to be able to judge whether doing it this way would achieve the desired effect.

ScottFreeCode commented 8 years ago

On a little further thought, let me put my query another way -- assuming, since it's being requested, that it must make sense somehow, and assuming, since it would be sort of hypocritical for a test library not to have tests, that we want to have some kind of tests for the generator handling that we can run (preferably automatically) to be sure that we've implemented it correctly and to be sure that future changes don't break it: What would some trivial test functions be that could demonstrate that generators and yield are doing what you want them to do in tests? (Beyond the mere fact that the yielding function returned by the generator is run, but not needing any additional 3rd-party libraries -- though a minimal stub instead would be great if some other function or object is needed.) Because that's what's motivating my concern -- sure, it might be trivial to have Mocha detect generator functions and run the thing they output rather than just running them, but the real trick is making sure whatever that's supposed to do is properly tested. (There also may be an issue of how to determine which environments support generators -- e.g. the old versions of Internet Explorer we've added tests for -- and prevent the generator tests from blowing up in those environments; I haven't looked into that yet, was hoping to first figure out whether I'll actually need to.)

antpaw commented 8 years ago

Hmmm it looks like I've missed to put my point across, I will try to do it again. It does not matter how useful generators are, or whats the best way to implement them to mocha. This is a topic for another discussion. What currently is broken in mocha is that you have the following behaviour:

it('test') // makes it pending
it('test null', null) // makes it pending
it('test false', false) // makes it pending
it('test undefined', undefined) // makes it pending
it('test Int', 5) // breaks with "TypeError: fn.call is not a function"
it('test Object', {}) // breaks with "TypeError: fn.call is not a function"
it('test String', 'test') // breaks with "TypeError: fn.call is not a function"

// passes
it('test Generator', function*() { throw new Error('oh no'); return (yield 'X'); });  

so just because you can fn.call something, doesn't mean its a function. That should be the first step, before discussing about how to implement Generators.

ajsharp commented 8 years ago

@ScottFreeCode @antpaw I'll open a new issue for discussion on adding support for generators, as a feature.

ScottFreeCode commented 8 years ago

So, I think I've figured this one out. It appears that something that either I missed when reading up on generators or else went unmentioned in the material I read was that the result of a yield expression (within the generator function) is whatever is passed to the next call to next. From there, there are libraries that wrap generators to take yielded promises and call the generator's next again with the resolved value in the promise's then, thus making = yield a sort of syntactic sugar for chained thens.

Now that I know that the generator functions would be wrapped to add this behavior (which explains why I didn't know of it from the mere generator explanations I'd read), I can see the usefulness -- the appeal, even, though it seems like it's more a stylistic than a functional difference from chained thens. What I'm not sure about is whether it's worth building the behavior into Mocha's tests, vs. using a function something like this: it(wrapGeneratorTest(function*(){/*test code here*/})) (where wrapGeneratorTest would take a test generator function and return a function that takes done and calls the test function with yield-promise wrapping but also with this bound properly if possible). Using such an intermediate wrapper function has advantages such as:

On the other hand, arguably the same thing goes for using a returned Promise as an alternative to done, but that's baked into Mocha's test-running functionality. Then again, the handling for returned promise borders on trivial (if the returned value is a promise, treat test as asynchronous and call the promise's .then(done, done)) and can have library shims for testing in environents that don't provide promises, so as a tradeoff that's much cheaper. Hmm...

In any case, now that I get it I'd be tentatively interested in either providing a function to wrap generator tests for yield-promise behavior like that, or else building it into the test functionality itself, depending on which is preferred by the rest of the @mochajs/core team. I also don't know whether users of existing third-party generator extensions to Mocha have any strong opinions on changing to use a wrapper explicitly, so I guess it'd be good to get some kind of poll going or something...? Oh yeah, and we should look at whether async/await is going to render it obsolete in the foreseeable future (hence why I qualified "interested" with "tentatively").

JoshuaKGoldberg commented 10 months ago

Lots of text for an issue whose last activity was 2017 🙃.

Oh yeah, and we should look at whether async/await is going to render it obsolete in the foreseeable future

Generators are pretty rare in userland code these days. They had a long moment with tools like redux-saga but it's been a while since I've seen them be used in a popular framework. I'm guessing the lack of activity on this issue (and low download counts of co-mocha and mocha-generators) means that the new async/await support has been enough for most users.

See also https://github.com/mochajs/mocha/issues/2024#issuecomment-197071878 & issues linked in there.

Closing as wontfix the same as the other generator support requests. But if someone is truly trying to test their generators/iterators/etc. with Mocha, please do file a new issue with our new feature request template. We'd be happy to take a look!

ScottFreeCode commented 10 months ago

Gotta admit it's amusing in hindsight watching myself try to figure out that generators were being used as async-await before we had async-await. There's an interesting bit of Node/JavaScript trivia from a decade ago that apparently I didn't know till it was almost obsoleted by async functions.

Wonder how commonly used generators are for other purposes at this point?

(I read a fascinating argument once against baking async-await into the language on the grounds that not only are generators able to do the same thing but they are more versatile since the augmented functionality of a generator could be all sorts of things and not just promise unwrapping, and implementing it as a language feature means only one of those possible functionalities gets privileged. I must have read that after trying to look at this issue.)