Macro testing and request module

MDN
#1

I’m working on an initial test for APIRef and have run into a snag. It seems as if part of the test framework, somewhere, is creating a namespace collision, replacing the request NPM module with its own code.

This can then be seen any time the MDN.fetchHTTPResource() function is called. An exception winds up being thrown; if you inspect that exception, you find this (pardon any line numbers that don’t quite line up; I’ve been adding bits of debug output code):

Error: TypeError: Object.defineProperty called on non-object
TypeError: Object.defineProperty called on non-object
    at Function.defineProperty (<anonymous>)
    at Object.get (/node_modules/sinon/lib/sinon/default-behaviors.js:184:16)
    at Object.proto.(anonymous function) [as get] (/node_modules/sinon/lib/sinon/behavior.js:202:12)
    at Function.get (/node_modules/sinon/lib/sinon/behavior.js:195:46)
    at to_cache (eval at compile (/node_modules/ejs/lib/ejs.js:549:12), <anonymous>:314:35)
    at /app/lib/kumascript/api.js:246:21
    at cls.get (/app/lib/kumascript/utils.js:73:9)
    at Object.cacheFn (/app/lib/kumascript/api.js:238:12)
    at Object.fetchHTTPResource (eval at compile (/node_modules/ejs/lib/ejs.js:549:12), <anonymous>:364:20)
    at Object.fetchJSONResource (eval at compile (/node_modules/ejs/lib/ejs.js:549:12), <anonymous>:292:31)
    at cls.subpagesExpand (eval at compile (/node_modules/ejs/lib/ejs.js:549:12), <anonymous>:90:28)
    at eval (eval at compile (/node_modules/ejs/lib/ejs.js:549:12), <anonymous>:52:24)
    at returnedFn (/node_modules/ejs/lib/ejs.js:580:17)
    at /app/lib/kumascript/templates.js:71:26

As you can see there, when the to_cache() function, found in fetchHTTPResource() in the MDN-Common.ejs macro file calls request.get(), expecting the call to go into the NPM module request, instead the call gets routed into Sinon, part of the test framework, even though it has not been stubbed or overridden intentionally in any way. That’s the line that starts “at Function get(...”.

Eventually the thing reaches Object.defineProperty(), which tries to do something that won’t work simply because the thing has wandered off into totally wrong code. Exception throws, and the macro call (and therefore the test) fails.

If I add to the initialization of the APIContext (in the APIContext.initialize() method in lib/api.js, a line to add request2: request to the object (that is, a reference to the request object with the name request2, then use request2.get(), that succeeds as expected. Thereby establishing there is a name conflict occurring.

This appears to mean that fetchHTTPResource() and its child fetchJSONResource() only work correctly when not testing. At least under the circumstances in which APIRef's test is running.

Does anyone know how I can fix this properly? Or any ideas how to avoid it?

#2

I had a similar issue with this, I solved it by using a Sinon stub:

itMacro("…", macro => {
    macro.ctx.page.subpagesExpand = sinon.stub();
    …
});

See also:

1 Like
#3

I think you’ve already got this answer @Sheppy on https://github.com/mdn/kumascript/pull/996, but I want to get it here for future reference.

@exe-boss is pointing in the right direction. The error is not great, but the problem is that request has been stubbed for all tests and will fail unless you set it up with the expected call. This is intentional - unit tests should not be loading data from an external service. It is setup in tests/macros/utils.js along with some other boilerplate.

Why should you avoid network requests in unit tests? I could write a whole essay on this, and I assume someone has, but I’m having trouble finding a good blog post. This post is typical, assuming you know why you want to avoid network requests. Some quick reasons:

  • Network requests are slow, so faking them makes tests fast
  • Network services are unreliable, so faking them makes tests reliable
  • Network services require setup, so faking them reduces test setup
  • It is hard to test a failed network service, but easy to fake a failed network service
  • A network response can change over time, but a fake response doesn’t change until you want to

The general process I use is:

  1. Find a simple macro call and the generated output on MDN
  2. Write a test that checks for that output.
  3. Run the test and get rid of that error. I think it would work to mock the Kuma API call, like something$children, or, like @exe-boss suggests, mock subpagesExpand. I would start with the real output, and then remove data until it just has the data the macro uses for this call.
  4. Repeat until the test fails because the output is different, rather than errors
  5. Change your output to match the test output, or find the input you forgot about.
  6. Look at the macro, find the branch you didn’t test, write a test for that
  7. When tests are complete, COMMIT
  8. Add features or change behavior in small steps, updating the test at the same time as the macro.

See the initial commit for jsxref tests to see what the output of that process looks like, and the next commit which changes behavior and tests.

1 Like