Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added mvt-fixtures' real-world integration tests #6250

Merged
merged 4 commits into from
Mar 1, 2018

Conversation

brunoabinader
Copy link
Member

Adds real-world integration tests with data coming from https://github.com/mapbox/mvt-fixtures:

  • real-world/bangkok
  • real-world/chicago
  • real-world/nepal
  • real-world/norway
  • real-world/sanfrancisco
  • real-world/uruguay

Changes were made in the integration test HTTP server:

  • Added extra /node_modules path mount (for finding .mvt files from the mvt-fixtures node module)
  • Resolve local:// URLs also for source tiles field (if used instead of url)

The tests' styles are all based in basic-v9, with slight changes:

  • In all tests, the mapbox source tiles field is unique for each test (because the .mvt files are stored in separate folders)
  • For Bangkok, the fonts were all replaced to NotoCJK because these provide the extra glyphs
  • For all tests containing terrain, a custom contour layer was added (missing in basic-v9 style)

@brunoabinader
Copy link
Member Author

Possible improvements include:

  • Hillshading layers
  • Complex styles (e.g. similar to streets-v9, but openly available)

@brunoabinader
Copy link
Member Author

brunoabinader commented Feb 28, 2018

I've run these tests in GL native, and so far only tests that show road labels have failed - see e.g. Bangkok:

GL JS GL native diff
bangkok-gl-js bangkok-gl-native bangkok-diff

As pointed out by @ChrisLoer, there is a similar issue tracking label positioning diffs in mapbox/mapbox-gl-native#10412.

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
There are several existing tests that use local://../../node_modules/... URLs. Please update these to use local://node_modules/... as well.

@brunoabinader
Copy link
Member Author

@jfirebaugh I ended up using Map's transformRequest instead of transforming URLs on the HTTP server - much simpler, and also enabled us to clean up the remaining ../../node_modules entries.

@brunoabinader brunoabinader force-pushed the mvt-fixtures-real-world branch 2 times, most recently from 1d15db7 to a87d1bc Compare March 1, 2018 10:14
@mapsam
Copy link
Contributor

mapsam commented Apr 13, 2018

@brunoabinader just seeing this for the first time - awesome work! Lemme know if you have any issues with mvt-fixtures, happy to help with updates.

@brunoabinader
Copy link
Member Author

Thanks @mapsam :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants