Adjust tests for mostly Placeholder changes #438
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dianashk
reviewed
Sep 28, 2017
test_cases/encoding.json
Outdated
| "in": { | ||
| "text": "pärnu, estonia" | ||
| }, | ||
| "expected": { | ||
| "properties": [ | ||
| { | ||
| "name": "Parnu", | ||
| "region": "Pärnumaa", | ||
| "region": "Pärnu County", |
There was a problem hiding this comment.
we shouldn't mix the english names with those special characters. if we selected this test because we wanted to see appropriate accents in the results then it should be ok to add the language parameter that exposes the accents in the results.
There was a problem hiding this comment.
Good suggestion. I've changed the output language to Estonia, and created another acceptance test to track the related sorting issue.
orangejulius
force-pushed
the
placeholder-new-data
branch
from
ee1a3e4 to
7456c21
Compare
Malmo now comes back in English as Malmo, while specifying `lang=sv` keeps the umlaut. Capture both cases in a test.
Some WOF records have added trailing spaces to names. Hopefully it will be resolved soon.
Labels are generally composed of properties already tested, and when the label itself is not whats being tested, generally should not be tested.
This is necessary due to a sorting bug, but for this test in particular we only care about the encoding aspect
Placeholder does not yet handle Empires: ttps://github.com/pelias/placeholder/issues/54
This was exposed due to our new admin lookup strategy and is tracked in pelias/wof-admin-lookup#156
orangejulius
force-pushed
the
placeholder-new-data
branch
from
7456c21 to
1efb626
Compare
|
Okay, this PR is ready to be looked at again and now brings us back to all passing tests on dev. |
These streets are now returned with the latest placeholder data!
dianashk
approved these changes
Sep 29, 2017
ghost
removed
the
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adjusts our acceptance tests for all recent Placeholder changes.
In general, many translations have been updated and in many cases English translations have been added, often ones that have no accents or diacriticals.
A few regressions have happened, mostly around sorting such as pelias/placeholder#55.
Two tests are temporarily marked failing because the names somehow had trailing spaces added, but both cases have been fixed via Boundary Issues and should resolve soon.
With this, combined with the changes in #434 and #437, which handle PIP and empire/dependency related issues respectively, we should be pretty much back to zero failing tests.
This PR can be tested against Mapzen Search
devonly, as nowhere else has updated placeholder data