sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.63k stars 769 forks source link

sandbox.restore() doesn't works between two 'describe' (mocha Suite) #2473

Closed lunard-gruber-logistics closed 1 year ago

lunard-gruber-logistics commented 1 year ago

Hello to all, I have a problem to use the sandbox between two 'describe': basically the restore function seems not to restore the original functions, stubbed using the sandbox. The stubbed version still remains across the two 'describe' I also tried to clear the modules 'my-library' and 'sinon' from the 'require' cache, without success

Someone faced the same problem ?

import chai, { expect } from 'chai';
import chaiHttp from 'chai-http';
import { AuthHelper } from 'my-library';
const sinon = require('sinon');

chai.use(chaiHttp);

describe('suite1', () => {
  let serverApp: any;
  let sandbox: any;

  afterEach(() => {
    sandbox.restore();
  });

  before(()=>{
      sandbox = sinon.createSandbox();
  });

  beforeEach(() => {
    sandbox.stub(AuthHelper, 'authExpressMiddleware').callsFake((options: any) => (req: any, res: any, next: any) => {
      console.log("Stubbed authExpressMiddleware");
      return next();
    });
    serverApp = require('myExpressApp');
  });

  it('test1', (done) => {

    chai.request(serverApp)
      .get(`my-action`)
      .end((err, res) => {
        expect(res.status).to.equal(200);
        done();
      });
  });
});

describe('suite2', () => {
  let serverApp: any;
  let sandbox: any;

  afterEach(() => {
    sandbox.restore();
  });

   before(()=>{
      sandbox = sinon.createSandbox();
  });

  beforeEach(() => {
    // PROBLEM: the test should pass (return 401) because I don't stub the auth middleware, 
   // but the test fails because the previous stub is called (I see the message "Stubbed authExpressMiddleware" also in the suite2)

    serverApp = require('myExpressApp');
  });

  it('test1', (done) => {
    chai
      .request(serverApp)
      .get(`my-action`)
      .end((request, res) => {
        expect(res.status).to.equal(401);
        done();
      });
  });
});
lunard-gruber-logistics commented 1 year ago

I think this is a duplicated Issue: #2065 , right ?

fatso83 commented 1 year ago

Hi, Lunard. I assume you have read the entirety of #2065 , in which case you should be able to tell if this is a duplicate or not. If this is in fact a duplicate, then we can close this, as #2065 was closed due to every reported case not being an issue with Sinon, but with how it was being used.

I have not spent time analyzing and running your code, but please read through #2065 carefully to see why the reported cases were discovered only to be usage errors (I saw 3 such explanations from myself when browsing that issue now). If you with that knowledge can have a fresh look at your own code and not spot any issues, I will try and do a deep dive when time allows.

fatso83 commented 1 year ago

My prime suspect would be this:

  beforeEach(() => {
    sandbox.stub(AuthHelper, 'authExpressMiddleware').callsFake((options: any) => (req: any, res: any, next: any) => {
      console.log("Stubbed authExpressMiddleware");
      return next();
    });
    serverApp = require('myExpressApp');
  });

Essentially you are modifying global state shared by two tests and you need to be very certain how your test code runs and cleans up to be sure it works correctly.

fatso83 commented 1 year ago

I would really appreciate it if you could make a reproducible test case if I am to look into this. Just copy your own git repo and delete code until there is just the skeleton left.

lunard-gruber-logistics commented 1 year ago

@fatso83 sure, on Monday I will prepare a working example to prove my issue, thanks !

fatso83 commented 1 year ago

Never got a reproduction case. Closing as not reproducible