search

testdouble / quibble

Makes it easy to replace require'd dependencies.
94 stars 25 forks source link

Different behavior with Quibble 0.5.3 compared to Quibble 0.5.1 #20

Closed restfulhead closed 6 years ago

restfulhead commented 6 years ago

Not sure whether to post here or in testdouble.js. I'm experiencing some issues when using Quibble 0.5.3 compared to Quibble 0.5.1.

I'm using testdouble.js and I only noticed this, because it requires "quibble": "^0.5.1". Our tests with a package-lock.js on 0.5.1 work. A fresh npm install without the lock creates a dependency to 0.5.3. I think tesdouble.js should lock in the version, but that's another discussion.

I can't exactly pinpoint where things go wrong, but here's what happens with 0.5.3. We run tests with NODE_ENV=test mocha 'test/*.spec.js'. The interesting thing is, if I run the .spec files one by one they work. However, if I use the command above and they run after each other, the mocks of the second test spec return values from the mock in the first test spec!

I have

afterEach(() => {
    td.reset();
});

in both files. Any ideas?

searls commented 6 years ago

Since I maintain both libraries, I keep the version unlocked, because I wanted folks to get this bug-fix version of quibble (0.5.3) that I published over the weekend. It fixes a number of module resolution edge cases.

That said, it could either:

  1. Have introduced new bugs with it (wups), causing tests to fail that should be passing
  2. Be causing previously working tests to fail that shouldn't have been working, and can hopefully be fixed with some adjustment.

It's possible that this is a dupe of #19

@ruhkopf is there any chance you'd be able to provide a reproducible example of this issue? Because it sounds like a test pollution problem, I'm concerned it may be difficult to narrow down otherwise.

restfulhead commented 6 years ago

Ok, that makes sense. I don't think it's related to #19. Don't have a simple test case at hand that I could share. But I'll try to create one.

searls commented 6 years ago

If you're able to create a failing test or a reproducible example, that'd be a terrific help, thank you!

restfulhead commented 6 years ago

Luckily I was able to reproduce it. Thanks for your earlier assessment, it was spot on! It was doing something that shouldn't have worked and I think it may in fact be some what related to #19.

Failing tests: https://github.com/ruhkopf/quibble20 Successful tests: https://github.com/ruhkopf/quibble20/tree/quibble-0.5.1

One of the test specs had a td.replace in before(). However, before() was accidentally defined outside of the describe function. It was therefore invoked for all test files. And I referenced it without the js extension: td.replace('../lib/testlibA', testLibAMock). Then in the other test spec I do a require('../lib/testlibA.js').

My guess: This worked with quibble 0.5.1, because it was treating ../lib/testlibA and ../lib/testlibA.js as not the same module, so the accidental before() had no side effects. However, with 0.5.3 these are treated the same, which probably they should. So what I defined in before kicks in.

Does this makes sense? In any case, this was my bad. Moving the before callbacks to the proper location (inside describe) fixes the issue I was facing. This ticket be closed.

searls commented 6 years ago

Yep, that sounds like exactly what happened. Glad that you were able to identify the root cause as quickly as you did, and pardon the interruption!