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

Alphanumeric housenumber test improvements #568

Merged
merged 6 commits into from
Jan 22, 2025

Conversation

orangejulius
Copy link
Member

A little cleanup to our alphanumeric housenumber tests to go with pelias/api#1683 / pelias/query#139.

The major changes are:

Minor changes:

  • renaming the test suite to be prefixed by search_. This is a bit random but I think our test suites should be for a specific endpoint. Most already are, so this just adds to that trend.
  • Adjust normalizers to make matching housenumbers easier, in this case between 14/a in the results and 14a in the test

After pelias/api#1683 the only test we still fail on is Via del Ponticello 38/2 Trieste italy, where we return a place with housenumber 2, so there's very nice improvement here.

We don't have any firm conventions, but having an endpoint name in the
suite name seems to make sense.
For most search queries, the client reading the results may be doing so
programatically. Not getting the right result sorted first is almost as
bad as not returning it at all.
I don't understand any Cryillic, but it looks like this test has not had
a streetname that matches the name in OSM for a while.

This appears to be the expected record:
https://www.openstreetmap.org/way/431615462
@missinglink
Copy link
Member

Added some additional test cases from pelias/pelias#810 (comment)

@missinglink missinglink merged commit b47b3c9 into master Jan 22, 2025
1 check passed
@missinglink missinglink deleted the alphanumeric-housenumber-test-improvements branch January 22, 2025 10:33
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