pelias / api

HTTP API for Pelias Geocoder
http://pelias.io
MIT License
218 stars 162 forks source link

fix: test case fails for diffPlaces #1644

Closed wasi-m closed 1 year ago

wasi-m commented 1 year ago

Describe the bug

The Test case fails for the diff Places

Steps to Reproduce

Clone dev branch, and run the tests

PELIAS_CONFIG=./test/test-pelias-config.json node --trace-uncaught test/unit/run.js | npx tap-dot

Expected behavior

Should check for grolmanstrasse

Environment (please complete the following information):

Pastebin/Screenshots

image
michaelkirk commented 1 year ago

This test is failing for me too.

It seems like the underlying remove-accents package is implicated.

The test is depending on behavior from this change: https://github.com/tyxla/remove-accents/issues/12 included first in release v0.4.2.

But that behavior was reverted in 0.4.4 released in November '22. See https://github.com/tyxla/remove-accents/pull/31

I don't really know what behavior is better. But if we want to just delegate that decision to remove-accents, then as well as updating this test, it might make sense to bump the minimum version of remove-accents to 0.4.4 to keep everything coherent.

missinglink commented 1 year ago

Hi, thanks for tracking this down, I alerted the author in the linked issue above.

orangejulius commented 1 year ago

I just merged https://github.com/pelias/api/pull/1655 to pin to a version of remove-accents that operates how our unit tests expect and we believe to be correct. Sounds like we can undo that pin in the near future though, as remove-accents may revert the change that broke our tests. Until then tests are passing again for everyone :)