conveyal / browsochrones

Create isochrones and accessibility images in the browser
MIT License
6 stars 2 forks source link

Add tests #87

Closed trevorgerhardt closed 7 years ago

codecov-io commented 7 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (dev@dfc6a3e). Click here to learn what that means. The diff coverage is 74.28%.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev      #87   +/-   ##
======================================
  Coverage       ?   62.99%           
======================================
  Files          ?       16           
  Lines          ?      508           
  Branches       ?       88           
======================================
  Hits           ?      320           
  Misses         ?      163           
  Partials       ?       25
Impacted Files Coverage Δ
lib/index.js 0% <0%> (ø)
lib/isochrone-tile.js 0% <0%> (ø)
lib/worker-handlers.js 0% <0%> (ø)
lib/stop-tree-cache.js 93.33% <100%> (ø)
lib/propagation.js 100% <100%> (ø)
lib/surface.js 84.21% <100%> (ø)
lib/constants.js 100% <100%> (ø)
lib/grid.js 100% <100%> (ø)
lib/get-isochrone.js 100% <100%> (ø)
lib/utils.js 100% <100%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dfc6a3e...c81abe9. Read the comment docs.

mattwigway commented 7 years ago

One other thing: are we sure it's wise to remove all of the metadata (zoom, width, height, etc) from a surface? Then we just have giant arrays of values floating around and we don't know how to interpret them.

One other thought I had was that we have two ways to represent grids of data: surfaces for travel time and grids for opportunity counts. Maybe we should just eliminate surfaces altogether and return grids of travel time?

trevorgerhardt commented 7 years ago

The metadata for the surface is the same as the query, correct? Since we use the set of objects <query, origin, stopTreeCache, grid> to generate a surface we may want to manually tie them together.

You'll have to elaborate a bit more on what you mean for the feasibility of the second question. What would the drawbacks be to doing that? Would there be significant increases in speed, ease of use, storage, etc.?

trevorgerhardt commented 7 years ago

Another word on the visualizations.

This library was created before we started more robustly testing front end components and we avoided adding tests here due to the nature of the library and running the example was assumed to show you if you shot yourself in the foot. I am not against using these visualizations to test data and changes here, but I think a more robust __test__ suite should be implemented first that can be run on each check.

Also, if these visualizations are robust/useful would they not be useful in other applications that use browsochrones?

trevorgerhardt commented 7 years ago

@mattwigway I'd like to merge this code in and cut a release. Since all the tests are new it's coverage is much more robust then it was before. I would say let's merge what's in here now (barring any significant code issues) and cut a release. With future versions we can make the tests more robust.