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

Include a default mock/stub library #47

Open
bitpshr opened this issue Jun 11, 2013 · 34 comments
Open

Include a default mock/stub library #47

bitpshr opened this issue Jun 11, 2013 · 34 comments
Labels
effort-medium This may take a couple of days enhancement A new or improved feature priority-medium This should get done, but it's not a high priority
Projects

Comments

@bitpshr
Copy link
Contributor

bitpshr commented Jun 11, 2013

It can be tedious work with stubbed / mocked functions. Jasmine and others include a spy implementation to make this easier. Chai has a separate plugin for this at https://github.com/chaijs/chai-spies. Rather than making the user pull in this plugin manually, it may be nice to have Intern wrap it, perhaps as a loader plugin similar to the assertion interfaces.

@manoharank
Copy link

+1 for built-in spy library

@csnover
Copy link
Member

csnover commented Jun 26, 2013

If we are going to include something to do this, I’d prefer it to not be something that says “you are probably going to want something more robust” in its readme.

@Bjornej
Copy link

Bjornej commented Jul 4, 2013

A more robust library could be sinon.js in conjunction with sinon-chai which provides assertions for chai based on sinon.js.

I've already used it in the past and it integrates very well with chai.

@lbod
Copy link
Contributor

lbod commented Aug 5, 2013

+1 for sinon however we ran into issues with xhr2 when dojo 1.8 was released, though the maintainer seemed to be amenable to accepting patches, it looks like more changes have been made recently so it may work now

@RoystonS
Copy link

RoystonS commented Aug 5, 2013

Mocking (and other) libraries come and go. It'd be nice not to be forced to use any one particular type of mock library. Currently we use Sinon (with a tiny workaroudn for XHR2), pulled in via require, but right now there's nothing stopping us moving to another library, or migrating piece-by-piece as we like.

@lbod
Copy link
Contributor

lbod commented Oct 24, 2013

@csnover is there any update on this and have you any thoughts whether support for stubbing/mocking libraries is likely to be included in future?

@csnover
Copy link
Member

csnover commented Oct 24, 2013

No, there is no update. I have no new thoughts on whether or not this is likely to be included in the future.

@lbod
Copy link
Contributor

lbod commented Oct 24, 2013

Ok thanks, you should probably mark this enhancement as invalid then. At least it lets others know.

Also I suggest you update the comparison table on the README to include this, other libraries like Buster and Jasmine support stubs/mocks & spies

@csnover
Copy link
Member

csnover commented Oct 24, 2013

It’s not invalid, it’s just not done yet. My thoughts are unchanged.

@phated
Copy link
Contributor

phated commented Nov 18, 2013

chai-spies is really slim in features and didn't work for many use cases I had. I submitted a PR to Sinon.JS to add AMD (since it was never suggested before).

@phated
Copy link
Contributor

phated commented Nov 23, 2013

SinonJS accepted my PR for AMD support sinonjs/sinon@7bf869e

@jason0x43 jason0x43 modified the milestones: 1.9, 1.6 Mar 31, 2014
@devangnegandhi
Copy link

+1 for including sinon.js as a built-in for intern

@bryanforbes
Copy link
Member

The problem I've had with Sinon's AMD support is that it only really works in the built version, so you have to configure the loader to point to sinon/pkg/sinon. As long as we're using a version from npm, this probably won't be a problem, but it makes the package config a bit goofy.

@yagoferrer
Copy link

Do you guys have any documentation or example on how to use chai-spies or sinon.js with intern.js? thank you!

@jason0x43
Copy link
Member

I have an example project at https://github.com/jason0x43/intern-mocking-example that gives a couple basic examples using the sinon branch in intern. Both the branch and the example are still a work in progress, though.

@yagoferrer
Copy link

Thank you Jason! That's so cool! I'lll use your branch for my tests.

@csnover csnover modified the milestones: 2.2, 2.1 Jul 7, 2014
@stdavis
Copy link
Contributor

stdavis commented Sep 29, 2014

@csnover: How are people currently getting spy/mock functionality in intern without something like this? Seems like such a basic requirement for a testing framework that I'm surprised that intern has come this far without it. Am I missing something?

@kitsonk
Copy link
Contributor

kitsonk commented Sep 29, 2014

sinon now sometimes works properly without the built version, but I regularly have failures. It frustrated me so much I started kitsonk/sutabu. It does basic stubbing and spying. It isn't fully compatible with sinon (nor was it intended to be). Hopefully it can inspire some stuff for The Intern.

@neonstalwart
Copy link
Member

@stdavis stubbing and mocking is independent of running tests. that's not to say that intern couldn't include something but it explains how it's quite easy to get by without this functionality being in intern.

i use sinon in my tests but i've learned which parts not to use with AMD... one big hint with AMD is only use spies and stubs but not stub behaviors

i put a few days effort into bringing sinon into better shape for working via AMD and it should be usable from master of sinon. waiting for a new release and hoping it doesn't get accidentally broken but wouldn't be surprised if it does get broken since i don't have the bandwidth to watch the codebase like a hawk.

@jason0x43
Copy link
Member

@neonstalwart Just out of curiosity, what kinds of issues do stub behaviors cause with AMD?

@neonstalwart
Copy link
Member

@jason0x43 due to the way sinon/lib/sinon.js loads AMD dependencies in parallel there are potential race conditions. an example is that sinon.behavior may not be defined by the time this block of code is executed - https://github.com/cjohansen/Sinon.JS/blob/v1.10.3/lib/sinon/stub.js#L135-149

this issue usually reveals itself when you have something like sinon.stub().returns(...) and an error is thrown indicating that there is no returns method.

the general fix for this was to rewrite all the sinon modules to explicitly declare the other modules they depend on, create a method that defines the API of that module so that it can be called at a time which is appropriate for the way in which the code is being loaded, and finally, some code was extracted into a new module in order to break circular dependencies that existed in just about every module - they depended on sinon/lib/sinon.js for utility code defined there and sinon/lib/sinon.js depended on each of the other modules in order to generate the full sinon.* exported from sinon/lib/sinon.js

@neonstalwart
Copy link
Member

I should also point out that a built version of sinon, which would have all the dependencies available synchronously, has its own issues. in order to bundle 2 AMD libs as part of the distribution the build script creates a dummy define that is within scope of the source of those bundled external dependencies. unfortunately, it is also within scope of all the sinon modules which means that if you try to load the built distribution of sinon, the real AMD define is never called and so you can't get a reference to sinon as an AMD dependency - you need to reference the global instead.

the whole thing was a huge mess. @stdavis I think the question for me is how did sinon ever come so far in such a state 😜

@jason0x43
Copy link
Member

Ah, that. I've been using intern's order module to load all the pieces, which has worked well enough in my limited testing.

@neonstalwart
Copy link
Member

I've been using intern's order module to load all the pieces

i've been doing a similar thing for a quite a while too.

jason0x43 added a commit that referenced this issue Oct 27, 2014
When loaded by an AMD loader, sinon loads its dependencies
asynchronously using the loader's CommonJS compatibility mode. However,
sinon's dependencies are not meant to be loaded asynchronously, so there
are race conditions. This update uses a loader map to prevent sinon from
initially loading its dependencies. After the main sinon module is
loaded, the loader configuration is restored to its original value and
sinon's submodules are loaded in the proper order using Intern's order
plugin.

refs #47
@jason0x43
Copy link
Member

Just using intern!order didn't work so well after some slightly more complex tests, and I ran into the race condition mentioned by @neonstalwart. The most recent commit gets around that problem by using a loader map to mock sinon's dependencies when the main sinon module is loaded.

@neonstalwart
Copy link
Member

@jason0x43 have you tried to use sinon from master? apparently there should be a new release soon and i'd like to know if you've found anything broken. it would be best to address all the issues within sinon if possible.

@RoystonS
Copy link

We've actually found integrating Sinon with Intern to be very straightforward, and have been happily using the two together since June 2013 without any problems, using the standard 'built' Sinon distribution. We've a crazy-simple top-level sinon.js module, which works around Sinon's current AMD support:

define(["./sinon-dist/sinon.js"], function() {
    return sinon;
});

Tests that want Sinon simply pull that module in and use the resulting module value. If Sinon is changed so that it's more AMD-friendly later, we'll replace our module with a reference to that directly.

We used to have some nasty code that helped Sinon work with the XHR2 mechanism in Dojo, but they now work together out of the box. We also used to remove the global sinon variable, but we now leave it in place so that sinon-chai works.

Integrating sinon-chai used to be slightly tricky in Intern, but when Intern exposed some more bits of chai, that's straightforward too:

define(["intern/chai!", "third_party/sinon-chai/lib/sinon-chai"], function (chai, sinonChai) {
        chai.use(sinonChai);
        // Other bits: we also integrate Chai-as-promised and our own chai helpers too...
});

We're currently on Sinon 1.10.3, Intern 2.1.1 and Sinon-Chai 2.5.0. (We can't use Sinon-Chai 2.6.0 as it doesn't play nicely with Chai 1.9.1 from Intern.)

Hope this helps somebody: the integration of the existing public releases of Sinon and Intern doesn't have to be difficult.

@jason0x43
Copy link
Member

@neonstalwart It looks like 1.11.0 includes some improved AMD support (is there more in master?). I'll try it out. I was going for a stable interim solution since getting upstream changes into libraries can take a while, but it would certainly be ideal if sinon has already take care of the issue.

jason0x43 added a commit that referenced this issue Oct 28, 2014
Make use of sinon 1.11.0's much improved AMD support.

refs #47
@neonstalwart
Copy link
Member

@jason0x43 1.11.0 was released very shortly after i left my previous comment - so 1.11.x has everything that i was talking about in master.

@csnover csnover modified the milestones: 2.2, 3.0 Nov 17, 2014
@csnover csnover modified the milestone: 3.0 Apr 1, 2015
@csnover csnover added help-wanted effort-medium This may take a couple of days labels May 22, 2015
@indolering
Copy link

Has anyone taken a look at testdouble.js? It's got some commercial backing....

@mightyiam
Copy link

I did. I don't like that it doesn't allow mocking of external dependencies.

@indolering
Copy link

It sounds like they would accept a PR that provides that functionality. That being said, it sounds like Sinon is finally up-to-snuff....

@skitterm
Copy link

It would still be nice if this were shown in the "what can Intern do that others can't?" overview. I'm not suggesting that Sinon needs to be coupled with Intern, but it would be good to know where Intern stands compared to other libraries in terms of integration with a mocking/stubbing library. And/or a section in the docs about how to use Intern with the more common mocking/stubbing libraries (like there is for the section on integrating Intern with grunt/gulp).

@indolering
Copy link

Yeah, this is as much a documentation issue as a missing feature. Anyone feel like rolling up their sleeves and adding this to the user guide?

@jason0x43 jason0x43 added the priority-medium This should get done, but it's not a high priority label Apr 14, 2020
@jason0x43 jason0x43 added this to To do in Development May 28, 2020
jason0x43 added a commit to jason0x43/intern that referenced this issue Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-medium This may take a couple of days enhancement A new or improved feature priority-medium This should get done, but it's not a high priority
Projects
Development

No branches or pull requests