Skip to content

Conversation

@lucaswoj
Copy link

cc @jfirebaugh @ansis @jakepruitt @mapsam

There are a number of tests that began failing after the v2 upgrade due to reliance on undefined behaviour. These tests fall into two categories:

Failing Hidden Symbol Tests

These tests were relying on label placement. Changes in geometries, such as merging lines with identical properties, caused the label placement to shift enough to cause these tests to fail.

What the test was supposed to be

zuksgmbadddbswl7dzwlu7vn5el6g9 dh8mv4o676fbq pjrssjlivsld80r8pxwcn8b78dk83dhqcr7h8rx4or1ejn mtymllfkomp8pbnph20qfip8aaaaasuvork5cyii

What the test was after v2 upgrade

icyzvdcuruwaaaaasuvork5cyii

Other Failing Tests

Many other tests were failing because they were querying the edge of rendered features, getting just enough of an antialiased pixel to return the expected results.

wp skkqwr4rmgaaaabjru5erkjggg

Bonus: expected.json Indentation

The query-tests/*/*/expected.json files now use 4 spaces for indentation, consistent with other JSON files in this repository.

@jfirebaugh
Copy link
Contributor

2 space indent is actually the standard. That's what all render test style.jsons use, due to it being gl-style-formats default.

@lucaswoj
Copy link
Author

It looks like

  • query-tests/*/*/style.json use 4 spaces (but they should have been using two, this tripped me up)
  • render-tests/*/*/style.json use 2 spaces per gl-style-format

I'll add a commit to make sure 2 spaces are used everywhere.

two spaces, no newline at the end of the file
@lucaswoj
Copy link
Author

I pushed a commit that makes all JSON files now use 2-space indentation (and provides a utility format JSON files in the future).

@jfirebaugh
Copy link
Contributor

👍 🚢

@lucaswoj lucaswoj merged commit d3dc856 into master May 17, 2016
@lucaswoj lucaswoj deleted the fix-query-id branch May 17, 2016 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants