Sinon causing issues again

[altano]

The new version of Sinon is throwing webpack for a loop again. This code:

    function makePublicAPI(require, exports, module) {
        module.exports = sinon;
        sinon.spy = require("./sinon/spy");
        sinon.spyCall = require("./sinon/call");
        sinon.behavior = require("./sinon/behavior");
        sinon.stub = require("./sinon/stub");
        sinon.mock = require("./sinon/mock");
        sinon.collection = require("./sinon/collection");
        sinon.assert = require("./sinon/assert");
        sinon.sandbox = require("./sinon/sandbox");
        sinon.test = require("./sinon/test");
        sinon.testCase = require("./sinon/test_case");
        sinon.match = require("./sinon/match");
    }

    if (isAMD) {
        define(makePublicAPI);
    } else if (isNode) {
        try {
            formatio = require("formatio");
        } catch (e) {}
        makePublicAPI(require, exports, module);         <========
    }

Is causing this warning:

WARNING in ./~/sinon/lib/sinon.js
Critical dependencies:
365:22-29 require function is used in a way, in which dependencies cannot be statically extracted
 @ ./~/sinon/lib/sinon.js 365:22-29

It's not obvious to me how to work around this. Any of you have any bright ideas?

(the line number indicated in the error, 365, is clearly wrong, and the line with the arrow is actually the offending code)

[fresheneesz]

I would guess that webpack is detecting that the require function is being passed into makePublicAPI, and is saying that it doesn't know how to handle that case.

To support this case, webpack would need to locate functions that require is passed into and parse those using the correct parameter name to find the requires. Its certainly doable, are you up to the task?

The other option is to change sinon so that its not using this particular pattern to support both AMD and CommonJS.

[jhnns]

sinon is our special friend... ^^

[sokra]

😢 just use UMD...

[altano]

Oh dear, I took a look at what it might take to switch it over to UMD and stumbled on this. What on earth? Not sure I'm going to want to tackle that immediately...

Anyone know a good SinonJS alternative? I've found the API hard to wrap my head around anyway, with all the extremely similar but not exactly the same functionality.

[fresheneesz]

Oh hm, Sinon.js is a test framework. I actually spent quite a bit of time writing deadunit, which provides a test API and nice pretty test output for node.js and the browser. It doesn't have any functionality for creating spies, but that's ok because using test spies is bad practice

[mzgoddard]

I bumped into this yesterday. For now I'm using the script-loader to work around this.

@fresheneesz Sinon.js isn't a test framework, it's a library to be used in testing. I'm not sure how that stack-overflow question demonstrates spies being a bad practice, it looks more like a conversation about a design that is hard to test.

[fresheneesz]

Ah, my mistake.

Well, the chosen answer in that stackoverflow question describes that test spies mean you're not doing black-box testings. The kind of so-called "white-box" testing that uses spies and mocks is orders of magnitude more work to write, not only because there is so much more test-code to write, but any time the implementation is changed, you must also change the tests (even if the interface remains the same). And this kind of testing is also less reliable than black-box testing, because you need to ensure that all that extra test code is correct, and while you can trust that black-box unit tests will fail if they don't match the interface, you can't trust that about code overusing mocks because sometimes the test doesn't even test much real code - only mocks. If the mocks are incorrect, the likelyhood is that your tests will succeed, but your code is still broken.

Anyone that has experience with white-box testing can tell you they're a pain in the ass to write and maintain. Coupled with the fact that they're less reliable, white-box testing is simply far inferior in most cases.

[mzgoddard]

That as an argument for how spies and mocks can be misused makes way more sense.

[jhnns]

I know this is off-topic, but personally I use Grey-box-testing. I know "something" about the implementation, but I don't really test everything "behind the scenes".

A common use-case for spies is that you have several functions calling a particular function. By just testing this particular function and by using spies in all other cases I can keep all tests simple and expressive. Otherwise I needed to duplicate all tests.

Testing something "behind the scenes" should be an exception, but sometimes it actually leads to cleaner and more maintainable tests.

[fresheneesz]

You always want to test your interface. If you're substituting testing your public interface with testing some inner function, I'd say that's the wrong move, no matter how significant that inner function is. If you have duplication in your tests, you can always take advantage of that symmetry - factor it out into common functions.

[luxx]

May or may not be helpful, but I ran into this issue with karma + sinon + sinon-chai. I was requiring sinon directly, however using karma-sinon-chai eliminated the problem, probably because of how it requires the dependencies.

If you are unit testing and your object under test has a command defined that sends a message to another object, but has no other visible side effects, you do need to assert the message was sent, which is exactly what sinon allows you to do. Recommended reading: https://speakerdeck.com/skmetz/magic-tricks-of-testing-railsconf

[binarykitchen]

Any news on this one?

@mzgoddard Would you mind to share your script-loader here?

[elsigh]

Want.

[altano]

I'll leave this here for others... if you're depending on libraries for testing that use Sinon, you can't control what version of Sinon the library uses. So you might run into a problem where the version it uses can't even be used with script-loader. To deal with this:

• Download a release version of Sinon, e.g. http://sinonjs.org/releases/sinon-1.14.1.js
• Turn on noParse for sinon: that means webpack will not parse the file, it won't be minified, require will not work in the file, etc.
• Use NormalModuleReplacementPlugin to point require("sinon") to your local file instead of the NPM package.

Like so:

module: {
    noParse: [
        /sinon/
    ]
},
plugins: [
    new webpack.NormalModuleReplacementPlugin(/sinon/, __dirname + "/TestRunner/vendor/sinon-1.14.1.js")
]

You should not normally do this because the library you depend on might upgrade to a version of Sinon that isn't API compatible with your local copy, or you might depend on multiple packages that depend on varying versions of Sinon. This approach is totally circumventing both NPM and webpack. And for sure, NEVER use this approach in releasing code, only tests.

Perhaps once Sinon 2 is more prevalent, these issues will go away, but in the mean time...

[tomatau]

Right now I'm running webpack mocha tests over a babel loader and import sinon from 'sinon' works only when used in conjunction with sinon-2.0 in my packages.json:

"sinon": "git://github.com/cjohansen/Sinon.JS#sinon-2.0",

However this doesn't support sinon.useFakeTimers() which is very unfortunate.

Thanks @altano for your alternative although it looks like it may throw up a bunch of problems of it's own!

[kpdecker]

For my own setup (babel, sinon, chai, sinon-chai) I had to make a few alterations to @altano's local copy solution:

For useFakeTimers:

import 'expose?lolex!lolex';

Also need to make sure that this is excluded from the babel loader as noParse doesn't seem to impact loader execution.

For the final bit, needed to make sure that sinon-chai was treated as normal, meaning slight modification to @altano's regexs:

  module: {
    noParse: [
      /\/sinon.js/
    ]
  },
  plugins: [
    new webpack.NormalModuleReplacementPlugin(/^sinon$/, __dirname + '/vendor/sinon.js'),
  ]

This is multiple levels of yuck but it seems to work for now :(

[danielkcz]

How comes that sinon is working with browserify just perfectly? I just started using webpack for a new project (gulp, webpack, babel, mocha, karma), but issues like this are giving me a second thoughts if it's mature enough for a development.

For the record, I am not using sinon for any mocking or spying. I love its convenient anonymous spy which can be simply passed as a callback to my methods and then evaluate results with couple of assertions instead of making ugly function like this.

it('should...', () => {
    let called = 0;
    myMethod((data) => {
        if (called === 0) {
            expect(data.whatever).to.equal('test');
        }
        called += 1;
    });
});

With sinon I can it simply like this. It's much more readable with less hassle (and possible mistakes).

it('should..., () => {
    const spy = sinon.spy();
    myMethod(spy);
    expect(spy.firstCall).calledWithMatch({ whatever: 'test '});
});

Can you perhaps recommend me some other library that can do this?

[andrei-picus-hs]

I've created a fork of Sinon over at uberVU/Sinon.JS that doesn't include the AMD logic. Works perfectly with webpack.

[maikdiepenbroek]

@andrei-picus-hs You sir, are my hero!

[screendriver]

Same here. Is there an easy way to get Babel, Mocha, Chai, Sinon and Sinon-Chai running with webpack? I can't directly reference GitHub dependencies here so I can't use the @andrei-picus-hs repository unless it's deployed to npm. As well I am afraid that it will always behind the official Sinon Repo.

The Sinon 2.0 branch seems dead by the way.

[melbourne2991]

@screendriver I searched far and wide, doesn't seem like it - @andrei-picus-hs solution is your best bet:

devDependencies: {
    "sinon": "git+https://github.com/uberVU/Sinon.JS"
}

Using Sinon's 2.0 branch works as well, but doesn't include the util functions (like the fakeServer)

[screendriver]

Unfortunately as I mentioned earlier: I can't directly reference GitHub dependencies here in my company.

In the meantime I switched away from webpack to SystemJS and jspm.

[heldr]

It works for me just adding noParse rule on webpack and requiring the dist version instead.

Example:

import sinon from 'sinon/pkg/sinon';

[SpaceK33z]

Thanks @heldr, that seems to work! In my webpack.config.js, I added this:

{
    module: {
        noParse: [
            /node_modules\/sinon/,
        ],
    }
}

[graingert]

Because of peerigon/legacy-loader#1 I've actually switched to using:

npm install --save-dev sinon@next

[ericdfields]

+1 for @heldr's technique

[jbcpollak]

I tried this:

module: {
  noParse: [
     /\/sinon\.js/,
  ],
  resolve = {
    alias: {
      sinon: 'sinon/pkg/sinon',
    },
  }
}

But I get errors like this:

ERROR in ./~/sinon/pkg/sinon-1.7.3.js
Module not found: Error: Cannot resolve 'file' or 'directory' ./buster-event-emitter in ..../node_modules/sinon/pkg
 @ ./~/sinon/pkg/sinon-1.7.3.js 248:30-63

ERROR in ./~/sinon/pkg/sinon-1.6.0.js
Module not found: Error: Cannot resolve 'file' or 'directory' ./buster-event-emitter in ..../node_modules/sinon/pkg
 @ ./~/sinon/pkg/sinon-1.6.0.js 250:30-63

ERROR in ./~/sinon/pkg/sinon-1.7.3.js
Module not found: Error: Cannot resolve 'file' or 'directory' ./define-version-getter in ..../node_modules/sinon/pkg
 @ ./~/sinon/pkg/sinon-1.7.3.js 251:23-57

ERROR in ./~/sinon/pkg/sinon-1.6.0.js
Module not found: Error: Cannot resolve 'file' or 'directory' ./define-version-getter in ..../node_modules/sinon/pkg
 @ ./~/sinon/pkg/sinon-1.6.0.js 253:23-57

ERROR in ./~/sinon/pkg/sinon-1.7.3.js
Module not found: Error: Cannot resolve module 'buster-core' in ..../node_modules/sinon/pkg
 @ ./~/sinon/pkg/sinon-1.7.3.js 263:13-35

I only depended on sinon 1.7.3, so I'm not sure where the references to 1.6.0 come from.

[ericdfields]

@jbcpollak move resolve outside of module:

{
  module: {},
  resolve: {}
}

[bcowgill]

+1 for @graingert

thanks, that's the only thing that has worked for me. I was getting out of memory garbage collection errors I've never seen before with the other methods!

[aaronjensen]

fwiw, we had better luck using:

import 'expose?lolex!lolex'
import 'script!sinon/pkg/sinon'

(replace with require/webpack aliases/loader config/etc if you want)

[slavik57]

@heldr solution worked =] you are the man!

[optimatex]

thanks @graingert , only ur solution with npm install --save-dev sinon@next works for me

[ecrona]

I initially attempted to import sinon manually (evidently not needed) which caused this problem, so I just removed the import line at the top of the script and used it globally, and it seemed to work.

[timmyLan]

@heldr +1, works for me,THX

[graingert]

FYI sinon@next is very close to release: sinonjs/sinon#966

[shults]

  // webpack config here: 
  resolve: {
    root: [ path.resolve(__dirname) ],
    extensions: ['', '.ts', '.js'],
    alias: {
      sinon: `${__dirname}/node_modules/sinon/pkg/sinon.js`
    }
  },
  module: {
    noParse: [ /sinon\.js/ ],
    // ...
  }

// importing sinon from file.spec.ts
import * as sinon from 'sinon';

[graingert]

@shults just use sinon@next

[aweiher]

If anyone comes here for this error in combination with karma and phantomjs:

PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  ReferenceError: Can't find variable: require

Add Sinon as external in webpack config:

externals: ["sinon"]

(currently webpack 1.x and sinon 2.1.0 for me)