Skip to content

Fix flaky spec in geocoder#13820

Merged
alecslupu merged 1 commit intodevelopfrom
test/flaky-gecododer-fix
Jan 10, 2025
Merged

Fix flaky spec in geocoder#13820
alecslupu merged 1 commit intodevelopfrom
test/flaky-gecododer-fix

Conversation

@andreslucena
Copy link
Copy Markdown
Member

@andreslucena andreslucena commented Jan 9, 2025

🎩 What? Why?

Lately we've seen some flaky related to Geocoding happen a lot in CI.

This is the stacktrace:


  1) Decidim::Geocodable calls the Decidim geocoding utility and geocodes the resource
     Failure/Error: Geocoder.search(query, options)

     WebMock::NetConnectNotAllowedError:
       Real HTTP connections are disabled. Unregistered request: GET https://nominatim.openstreetmap.org/search?accept-language=en&addressdetails=1&format=json&q=Carrer%20del%20Pare%20Llaurador,%20113 with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Referer'=>'44.lvh.me', 'User-Agent'=>'Decidim/0.30.0.dev (My Application Name)'}

       You can stub this request with the following snippet:

       stub_request(:get, "https://nominatim.openstreetmap.org/search?accept-language=en&addressdetails=1&format=json&q=Carrer%20del%20Pare%20Llaurador,%20113").
         with(
           headers: {
       	  'Accept'=>'*/*',
       	  'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
       	  'Referer'=>'44.lvh.me',
       	  'User-Agent'=>'Decidim/0.30.0.dev (My Application Name)'
           }).
         to_return(status: 200, body: "", headers: {})

       ============================================================
     # ./lib/decidim/geocodable.rb:65:in `search'
     # ./lib/decidim/geocodable.rb:39:in `block in do_lookup'
     # ./lib/decidim/geocodable.rb:48:in `with_record'
     # ./lib/decidim/geocodable.rb:38:in `do_lookup'
     # ./spec/lib/geocodable_spec.rb:37:in `block (2 levels) in <module:Decidim>'

  2) Decidim::Geocodable when the address is invalid calls the Decidim geocoding utility and try to geocode the resource, but the result is [NaN,NaN]
     Failure/Error: Geocoder.search(query, options)

     WebMock::NetConnectNotAllowedError:
       Real HTTP connections are disabled. Unregistered request: GET https://nominatim.openstreetmap.org/search?accept-language=en&addressdetails=1&format=json&q=aaa with headers {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Referer'=>'44.lvh.me', 'User-Agent'=>'Decidim/0.30.0.dev (My Application Name)'}

       You can stub this request with the following snippet:

       stub_request(:get, "https://nominatim.openstreetmap.org/search?accept-language=en&addressdetails=1&format=json&q=aaa").
         with(
           headers: {
       	  'Accept'=>'*/*',
       	  'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
       	  'Referer'=>'44.lvh.me',
       	  'User-Agent'=>'Decidim/0.30.0.dev (My Application Name)'
           }).
         to_return(status: 200, body: "", headers: {})

       ============================================================
     # ./lib/decidim/geocodable.rb:65:in `search'
     # ./lib/decidim/geocodable.rb:39:in `block in do_lookup'
     # ./lib/decidim/geocodable.rb:48:in `with_record'
     # ./lib/decidim/geocodable.rb:38:in `do_lookup'
     # ./spec/lib/geocodable_spec.rb:57:in `block (3 levels) in <module:Decidim>'

I'm not able to reproduce this locally, but I think the problem lies in the User-Agent, where it's looking for the string "Ruby" and finding other string ("Decidim/XXX"). This PR tries to fix this flaky by relaxing this check.

A new theory! I think the problem lies in another file, as we're resetting the Geocoder mocks, and that makes that this other stub is disabled. Let's see what the CI says 🤞🏽

📌 Related Issues

Testing

As it is a flaky that can't be reproduced locally (or at least I can't do it), I'll re-run the relevant job ("[CI] Core - Libs") at least 5 times to see if it happens again.

♥️ Thank you!

@andreslucena andreslucena added flaky spec type: fix PRs that implement a fix for a bug labels Jan 9, 2025
github-actions[bot]
github-actions bot previously approved these changes Jan 9, 2025
github-actions[bot]
github-actions bot previously approved these changes Jan 9, 2025
@andreslucena andreslucena changed the title Relax geocoder stub request in tests Fix flaky spec in geocoder Jan 9, 2025
@andreslucena andreslucena marked this pull request as draft January 9, 2025 16:11
@andreslucena
Copy link
Copy Markdown
Member Author

I think I solved this one 🥳

image

Mind that the first failures are actually related to a flaky in "[CI] Core - System", that fails a lot too. I'm trying to kill that one in #13827

This can be reviewed

@andreslucena andreslucena marked this pull request as ready for review January 10, 2025 11:40
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I remember trying to fix this locally, and had issues reproducing it. Will test directly in CI.

@alecslupu
Copy link
Copy Markdown
Contributor

@andreslucena , i am not 100% we should backport this, so i will merge only against develop ( not adding labels )

@alecslupu alecslupu merged commit abb37b7 into develop Jan 10, 2025
@alecslupu alecslupu deleted the test/flaky-gecododer-fix branch January 10, 2025 12:39
@alecslupu alecslupu added release: v0.28 Issues or PRs that need to be tackled for v0.28 release: v0.29 Issues or PRs that need to be tackled for v0.29 labels Jan 13, 2025
entantoencuanto added a commit that referenced this pull request Jan 14, 2025
* develop: (27 commits)
  WCAG navigation submenu (#13796)
  Update gem dependencies (part 3) (#13849)
  Prevent notifications for deleted users (#13812)
  Retries NPM installation a couple times to prevent network timeouts (#13831)
  Update gem dependencies (part 2) (#13839)
  Enhance signature pdf export (#13778)
  Fix HexaPDF dependency (#13834)
  Fix flaky spec in authentication (#13827)
  Merge upload field for documents and image on proposal admin form (#13735)
  Update gem dependencies (#13835)
  Upgrade erb_lint to 0.8.0 (#13833)
  Fix flaky spec in geocoder (#13820)
  Refactor modules mounting routes (#13294)
  Upgrade check-spelling action (#13825)
  Add missing images in the custom registration emails from meetings (#13632)
  Add missing translations (#13793)
  Fix proposal map performance with hundreds of markers (#13798)
  Create multiple surveys within same Survey component (#13420)
  Accountability bulk actions (#13730)
  Improve UI for sorting options on comments (#13670)
  ...
antopalidi pushed a commit to openpoke/decidim that referenced this pull request Feb 12, 2025
andreslucena added a commit that referenced this pull request Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flaky spec module: core module: dev release: v0.28 Issues or PRs that need to be tackled for v0.28 release: v0.29 Issues or PRs that need to be tackled for v0.29 type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants