Skip to content

Disable reverse geocoding in directions test#6963

Merged
tomhughes merged 1 commit intoopenstreetmap:masterfrom
hlfan:no-nomatim-tests
Apr 8, 2026
Merged

Disable reverse geocoding in directions test#6963
tomhughes merged 1 commit intoopenstreetmap:masterfrom
hlfan:no-nomatim-tests

Conversation

@hlfan
Copy link
Copy Markdown
Collaborator

@hlfan hlfan commented Apr 4, 2026

These unnecessary test failures have been bugging me. The solution doesn't need to be that oversimplified tho.

@hlfan hlfan force-pushed the no-nomatim-tests branch 2 times, most recently from 5f39e85 to f211c22 Compare April 4, 2026 19:46
@hlfan hlfan marked this pull request as ready for review April 4, 2026 20:07
@pablobm
Copy link
Copy Markdown
Contributor

pablobm commented Apr 6, 2026

I'm not aware of this one. Do you have examples of breakage in CI?

@pablobm
Copy link
Copy Markdown
Contributor

pablobm commented Apr 6, 2026

OK so, if I understand correctly, the Nominatim requests are often not resolved quick enough. As a result the interface may not have updated and we are comparing the wrong things. To solve this you simply disable the Nominatim requests.

Why the delete(" ") though?

@tomhughes
Copy link
Copy Markdown
Member

Is it hitting the live nominatim then? Surely it should be stubbed in test?

@hlfan
Copy link
Copy Markdown
Collaborator Author

hlfan commented Apr 6, 2026

I'm not aware of this one. Do you have examples of breakage in CI?

Why the delete(" ") though?

I think somewhere an array is cast to a string, joining the coordinates only with a comma. Had a look but wasn't able to find anything.
You can look at this run if it helps you: https://github.com/openstreetmap/openstreetmap-website/actions/runs/23986025546

Is it hitting the live nominatim then? Surely it should be stubbed in test?

Before I started my crusade of getting PostGIS working, I think sometimes tests would complain about that...

@hlfan hlfan force-pushed the no-nomatim-tests branch from f211c22 to 14c5431 Compare April 6, 2026 19:56
@pablobm
Copy link
Copy Markdown
Contributor

pablobm commented Apr 7, 2026

Before I started my crusade of getting PostGIS working

So there ARE issues 😄 Care to share your problems at #6713, please? 🙂

Comment thread test/system/directions_test.rb Outdated
@hlfan hlfan force-pushed the no-nomatim-tests branch from 14c5431 to 3f5093c Compare April 7, 2026 15:01
Comment thread test/system/directions_test.rb
@tomhughes
Copy link
Copy Markdown
Member

Looks good to me, and I'm fed up with randomly hitting that failure, so thanks.

I wonder if we should be thinking about a general stub to block external accesses in system tests?

@tomhughes tomhughes merged commit 7d342b9 into openstreetmap:master Apr 8, 2026
14 of 15 checks passed
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.

4 participants