search

mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.04k stars 2.21k forks source link

Mock module dependencies in unit tests #2685

Closed anandthakker closed 8 years ago

anandthakker commented 8 years ago

In the unit tests, I've encountered one place--and I suspect there are more--where the only feasible way to do the test is to assert something about some private _-prefixed state variable.

In the example linked above, this could be avoided by mocking map.js's reference to Style and then checking that the correct parameter is passed. This would make the test less brittle and also make it a much more direct test of the behavior it's supposed to be validating.

Something like proxyquire or rewire ought to make this doable.

lucaswoj commented 8 years ago

I am open to giving this a try. We have some tests that are unnecessarily awkward due to the inability to mock out an entire module.

Thoughts @tmcw @jfirebaugh @mourner?

jfirebaugh commented 8 years ago

Are we talking about mocking require, as @anandthakker initially framed it, or using dependency injection, as @lucaswoj retitled the issue? To me, dependency injection would mean not using require('style') in map.js, and instead passing a Style object (or mock) to the constructor and setStyle. I don't think we should do that. But trying out proxyquire or rewire sounds good.

anandthakker commented 8 years ago

dependency injection would mean not using require('style') in map.js, and instead passing a Style object (or mock) to the constructor and setStyle. I don't think we should do that.

👍 traditional DI would introduce significant architectural overhead of (in my opinion) dubious value

lucaswoj commented 8 years ago

Ah! I misunderstood the concept of dependency injection. Re-un-titling.

lucaswoj commented 8 years ago

this was done in https://github.com/mapbox/mapbox-gl-js/pull/2667