search

NYCPlanning / labs-applicantmaps

5 stars 5 forks source link

"Don't mock what you don't own" #550

Open allthesignals opened 5 years ago

allthesignals commented 5 years ago

We violate "don't mock what you don't own" by mocking ember-mapbox-gl, mapbox-gl, and mapbox-gl-draw.

The fix is to wrap all interactions with these dependencies into wrapper components. Then, we stub the wrapper components.

allthesignals commented 5 years ago

so I would do this differently I don't know much about the the draw part of this my tactic would be to make a static method MapDrawActions.deletePoint() which is passed in everything it needs call that from the handler test that the handler calls that with the correct arguments, then test the static handler by using the arguments as dependency injection the static method should take arguments of your own design, not an object constructed by mapbox or draw, or whatever so all interaction with mapbox api is in the handler the layer you mock is the one you own

allthesignals commented 5 years ago

This https://github.com/NYCPlanning/labs-applicantmaps/pull/552 PR will help us get closer to creating better mockable interfaces for these dependencies.

I think the first step is to understand exactly all the interactions that happen through the mapbox-gl and labs-map layers. Then we can create better interfaces for those things in the source code. This'll be better for testing in the long-run.