testdouble / quibble

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

Update documentation for adding `quibbble.reset()` #25

Open thestrugglingblack opened 5 years ago

thestrugglingblack commented 5 years ago

Hey @searls ! Finally got a chance to use your library. I was writing a test for one of my functions in my company's CLI. I used quibble to help me stub out a module and it worked as planned (Thanks a million) However when I tried executing a full test and lint through the project it, I received this error:

atlas-installer/node_modules/chalk/index.js:72
ansiStyles.color.closeRe = new RegExp(escapeStringRegexp(ansiStyles.color.close), 'g');
                                                                          ^

TypeError: Cannot read property 'close' of undefined
    at Object.<anonymous> (tlas-installer/node_modules/chalk/index.js:72:75)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Module._load (module.js:497:3)
    at Function.fakeLoad [as _load] (atlas-installer/node_modules/quibble/lib/quibble.js:90:12)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)

The test runs this command tape test/*.test.js && eslint *.js */*.js test/*/*.js. My colleague @whyvez had encountered the same issue but was able to comb through the code to find the resolution. It was quibble.restore(). I didn't come across in this documentation. Is it possible if we can update the readme.md with that line to help the next user?

searls commented 5 years ago

Thanks for sharing this! I'm sorry but other than the broad strokes of what you're saying happened, I don't think I understand the issue. Is there a way to reproduce the failure or explain the fix more concretely?

I want to be sure I understand the root cause before updating the docs

thestrugglingblack commented 5 years ago

The service is on node v.8 We are using

quibble('../package.json', { version: '1.1.1' }); const makeEndpointRequest = require('../lib/make-endpoint-request');

tape('[ makeEndpointRequest ] request returns 200', (t)=> { nock('https://localhost:3000', { reqheaders: { cli-version' : '1.1.1' } }) .get('/v1/offline/data001') .query({ 'access_token': 'TOKEN_HERE' }) .reply(200 );

offline.get('https://localhost:300/v1/offline/data001?access_token=TOKEN_HERE', (err, res)=> { t.error(err); t.deepEqual(res.statusCode, 200); t.equal(res.request.headers['cli-version'], '1.1.1'); }); t.end(); }); quibble.reset(); // <== had to add this


and the module we are testing is setup like this:

'use strict';

const request = require('request'); const { version } = require('../package.json');

const headers = { 'cli-version': version };

module.exports.get = function(url, callback) { request.get({ url: url, json: true, headers }, callback); };


With the test, I just wanted to verify that it was returning the correct header, so I had to stub out the package.json file. Using that file to pull in the version of the cli tool. When I didn't have `quibble.restore()` it made the error message in the first post. But the minute I added it in, it resolved and the test were able to execute and pass. Does this help give further context?
searls commented 5 years ago

A-ha! I see now. And all our readme does is say:

A reset() method that undoes everything, intended to be run afterEach test runs

My apologies. We only use quibble in the context of https://github.com/testdouble/testdouble.js td.replace() function, and its docs are pretty over-the-top in telling users to call td.reset() in an after-each hook of their test lib.

Yeah, I'd welcome an update to the README that said this. Thank you!

thestrugglingblack commented 5 years ago

Oh I see! I'll make a PR for an update in the quibble readme! Thanks for getting back to me on this!