Skip to content

Conversation

@bshand
Copy link
Contributor

@bshand bshand commented Jan 30, 2025

This PR attempts to get ndr_error tests running again (pre-bootstrap 5 upgrades to ndr_ui) and identify the cause of the recent brittle tests like the following (https://github.com/NHSDigital/ndr_error/actions/runs/12950940910/job/36217578005?pr=32 on PR #32):

Error:
ErrorViewingTest#test_should_be_able_to_purge_logs:
Selenium::WebDriver::Error::WebDriverError: aborted by navigation: loader has changed while resolving nodes
  (Session info: chrome=132.0.6834.83)
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/response.rb:63:in `add_cause'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/response.rb:41:in `error'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/response.rb:52:in `assert_ok'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/response.rb:34:in `initialize'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/http/common.rb:103:in `new'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/http/common.rb:103:in `create_response'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/http/default.rb:103:in `request'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/http/common.rb:68:in `call'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/bridge.rb:685:in `execute'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/remote/bridge.rb:167:in `url'
    selenium-webdriver (4.28.0) lib/selenium/webdriver/common/driver.rb:160:in `current_url'
    capybara (3.40.0) lib/capybara/selenium/driver.rb:121:in `current_url'
    capybara (3.40.0) lib/capybara/session.rb:232:in `current_url'
    capybara (3.40.0) lib/capybara/queries/current_path_query.rb:21:in `resolves_for?'
    capybara (3.40.0) lib/capybara/session/matchers.rb:24:in `block in assert_current_path'
    capybara (3.40.0) lib/capybara/session/matchers.rb:75:in `block in _verify_current_path'
    capybara (3.40.0) lib/capybara/node/base.rb:84:in `synchronize'
    capybara (3.40.0) lib/capybara/session/matchers.rb:74:in `_verify_current_path'
    capybara (3.40.0) lib/capybara/session/matchers.rb:23:in `assert_current_path'
    capybara (3.40.0) lib/capybara/dsl.rb:52:in `call'
    capybara (3.40.0) lib/capybara/dsl.rb:52:in `assert_current_path'
    test/integration/error_viewing_test.rb:121:in `block in <class:ErrorViewingTest>'
    minitest (5.25.4) lib/minitest/test.rb:94:in `block (2 levels) in run'
    minitest (5.25.4) lib/minitest/test.rb:190:in `capture_exceptions'
    minitest (5.25.4) lib/minitest/test.rb:89:in `block in run'
    minitest (5.25.4) lib/minitest.rb:368:in `time_it'
    minitest (5.25.4) lib/minitest/test.rb:88:in `run'
    ndr_dev_support (7.3.1) lib/ndr_dev_support/integration_testing/flakey_tests.rb:32:in `run'
    minitest (5.25.4) lib/minitest.rb:1208:in `run_one_method'
    minitest (5.25.4) lib/minitest.rb:447:in `run_one_method'
    minitest (5.25.4) lib/minitest.rb:434:in `block (2 levels) in run'
    minitest (5.25.4) lib/minitest.rb:430:in `each'
    minitest (5.25.4) lib/minitest.rb:430:in `block in run'
    minitest (5.25.4) lib/minitest.rb:472:in `on_signal'
    minitest (5.25.4) lib/minitest.rb:459:in `with_info_handler'
    minitest (5.25.4) lib/minitest.rb:429:in `run'
    railties (7.0.8.7) lib/rails/test_unit/line_filtering.rb:10:in `run'
    minitest (5.25.4) lib/minitest.rb:332:in `block in __run'
    minitest (5.25.4) lib/minitest.rb:332:in `map'
    minitest (5.25.4) lib/minitest.rb:332:in `__run'
    minitest (5.25.4) lib/minitest.rb:288:in `run'
    minitest (5.25.4) lib/minitest.rb:86:in `block in autorun'

rails test test/integration/error_viewing_test.rb:110

[Edit: copied from comment below]
All our failures were on calls to assert_current_path, and I've written an override method to retry this up to 3 times. I've added the following to test/test_helper.rb:

ActionDispatch::IntegrationTest.class_eval do
  # assert_current_path is brittle with Chrome 132 on capybara 3.40.0
  # Retry up to 3 times on error
  def assert_current_path(path, **options, &optional_filter_block)
    failures = 0
    begin
      super
    rescue Selenium::WebDriver::Error::WebDriverError => e
      failures += 1
      if e.message.start_with?('aborted by navigation: loader has changed ' \
                               "while resolving nodes\n") && failures <= 3
        # puts "Retrying after failure #{failures}: #{e.class} #{e.message}"
        retry
      end
      raise
    end
  end
end

@jrochkind
Copy link

jrochkind commented Jan 30, 2025

I arrived here github searching for "aborted by navigation: loader has changed while resolving nodes"

You can find several other people are having this error in such a search (more are prob having it without mentioning the phrase in a github issue?) I started having it this week: I think sometime this week Github upgraded their runners to chomedriver 132, which is part of the repro?

While I am also using Rails and capybara, this thread to me is I think about selenium use without ruby being involved. https://groups.google.com/g/chromedriver-users/c/dhan8JFk1r4

I'm thinking it's a problem in chromedriver 132 and/or between chromedriver 132 and selenium specifically? I am not sure how to bring it to the attention of anyone who can do anything about it though -- apparently it doesn't effect everyone universally, but just some of us are unfortunate enough to hit it?

it's race-conditiony for me, re-running the CI it will pass eventually, but it is annoying.

i don't believe there is anything I can change in my own code to avoid it, the error is occuring to me on an expect(page).to have_current_path -- some race condition in however selenium checks the current page url, seems to me.

@bshand
Copy link
Contributor Author

bshand commented Jan 30, 2025

@jrochkind I have a locally working fix. All our failures were on calls to assert_current_path, and I've written an override method to retry this up to 3 times. I've added the following to my test/test_helper.rb:

ActionDispatch::IntegrationTest.class_eval do
  # assert_current_path is brittle with Chrome 132 on capybara 3.40.0
  # Retry up to 3 times on error
  def assert_current_path(path, **options, &optional_filter_block)
    failures = 0
    begin
      super
    rescue Selenium::WebDriver::Error::WebDriverError => e
      failures += 1
      if e.message.start_with?('aborted by navigation: loader has changed ' \
                               "while resolving nodes\n") && failures <= 3
        # puts "Retrying after failure #{failures}: #{e.class} #{e.message}"
        retry
      end
      raise
    end
  end
end

@jrochkind
Copy link

jrochkind commented Jan 30, 2025

Thanks @bshand that's clever, makes sense.

Have to figure out how to patch that into my own expect-style have_current_path instead.

Will slow down our already non-swift CI though!

I wish I could find evidence that selenium and/or chromedriver teams were aware of this and interested in a fix.

@bshand bshand merged commit 0f97104 into main Jan 30, 2025
18 checks passed
@bshand bshand deleted the feature/fix_tests_chrome_132 branch January 30, 2025 15:54
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.

3 participants