Sinon: master build failing

[mroderick]

We have merged #1461, which resulted in an interesting failure where Chrome would fail the tests. I tried to fix that with proxyquire-universal.

Now I have run into a problem with the webworker test failing.

The setup to get the web worker test going is not exactly my strongest suit.

Ping @mantoni @fatso83: do you think you could take a look at this and see if you can figure out what the problem might be?

[fatso83]

I can have a look at it. Is the proxyquire change checked in somewhere?

[mroderick]

Is the proxyquire change checked in somewhere?

It's in master, because only master is built using SauceLabs. Perhaps we should try turning it on for PRs as well?

https://github.com/sinonjs/sinon/blob/master/.travis.yml#L30

[fatso83]

AFAI remember, we couldn't turn it on for pull requests due to dangers in exposing the sauce lab credentials. As anyone can push a pull request where they can console.log(process.env.SAUCE_LABS_SECRET) we could risk loosing the key, which we have assured Sauce Labs we would not.

[mroderick]

Ok. Then let's just stick to master

[fatso83]

OK, I have played with this a while - without fixing it, and thought I could report back some findings to you, @mroderick.

As for the WebWorker error, this is basically about the new build not creating a sinon object on the global scope. You can see this from the webworker script that is being loaded (test/webworker/webworker-script.js):

    importScripts("../../pkg/sinon.js");

    var stub = sinon.stub().returns("worker response");

This will fail if sinon is undefined. To see that this has nothing to do with WebWorkers, I created this CLI test:

npm i && ./build.js \
&& echo 'console.log("sinon:",window.sinon)' >> append.js \
&& cat pkg/sinon.js append.js \
      | $(npm bin)/phantomic --phantomjs node_modules/phantomjs-prebuilt/bin/phantomjs 

For 40f1599 (before the proxyquire changes) it outputs:

sinon: [object Object]
sinon: [object Object]
sinon: [object Object]
sinon: [object Object]

No idea why it happens four times, but it shows that sinon is defined.

After the proxyquire changes it outputs:

sinon: undefined
sinon: undefined
sinon: undefined
sinon: undefined
sinon: undefined

Now sinon is undefined (still no idea why it it showing up _five_ times).

So basically, the proxyquire changes breaks the build :cry:

[mroderick]

I've removed the proxyquire-universal module in master again, so you can see the failing build in Chrome again

[fatso83]

OK, had a go at it. Seems to be fixed with this patch:

diff --git a/test/sinon-test.js b/test/sinon-test.js
index c5be2af..b8c60b6 100644
--- a/test/sinon-test.js
+++ b/test/sinon-test.js
@@ -2,6 +2,7 @@

 var assert = require("referee").assert;
 var hasPromise = typeof Promise === "function";
+var proxyquire = require("proxyquire");

 if (!hasPromise) {
     return;
@@ -12,7 +13,6 @@ describe("sinon module", function () {
         fakeNise;

     beforeEach(function () {
-        var proxyquire = require("proxyquire");

         fakeNise = {
             fakeServer: "47c86a4c-6b48-4748-bb8c-d853f999720c",

Thought it looked a bit funny to place it in the beforeEach, so moved it up with the others, and it worked. My guess is that the browserify transform doesn't deal with require() statements nested within runtime code. I don't know the innards anyway, so might be totally off. Oh, well, it works.

I'll test some more, and if all goes well, I'll push it up.