Project

General

Profile

Actions

Patch #42422

open

Use Capybara's assert_current_path in "log_user" steps to wait for page in ApplicationSystemTestCase

Added by Takuya Kodama 15 days ago. Updated 7 days ago.

Status:
New
Priority:
Normal
Category:
Code cleanup/refactoring
Start date:
Due date:
% Done:

0%

Estimated time:

Description

  1. Problem

The login step in the system test sometimes fails because the expected page load isn’t complete when the assertion is executed as follows.

$ cd ../redmine && RAILS_ENV=test bin/rails redmine:plugins:test NAME=full_text_search
# Running:

...............Capybara starting Puma...
* Version 6.6.0 , codename: Return to Forever
* Min threads: 0, max threads: 4
* Listening on http://127.0.0.1:43813
.[Screenshot Image]: /home/runner/work/redmine_full_text_search/redmine/tmp/screenshots/failures_test_keep_search_target.png
F

Failure:
FullTextSearch::SearchTest#test_keep_search_target [test/application_system_test_case.rb:79]:
--- expected
+++ actual
@@ -1 +1 @@
-"/my/page" 
+"/login" 

rails test plugins/full_text_search/test/system/full_text_search/search_test.rb:58

............SSS..........................................................................

Finished in 124.323414s, 0.8526 runs/s, 1.0778 assertions/s.
106 runs, 134 assertions, 1 failures, 0 errors, 3 skips

ref: https://github.com/clear-code/redmine_full_text_search/actions/runs/13889360454/job/38858640197

  1. Cause

The test was using assert_equal to verify the current path immediately after form submission.
Since assert_equal does not wait for the page to load fully, it leads to intermittent failures in environments where page rendering is slower.

  1. Solution

Replace assert_equal with assert_current_path in the login steps.
This change leverages Capybara’s built-in waiting mechanism, ensuring that the test only proceeds once the browser has navigated to the expected page.

ref: https://www.rubydoc.info/gems/capybara/Capybara%2FSessionMatchers:assert_current_path


Files

Actions #1

Updated by Takuya Kodama 15 days ago

I'm so sorry I should use `Code cleanup/refactoring` category this time.

Actions #2

Updated by Takuya Kodama 15 days ago

Previous patch couldn't handle the path with query parameters.
Adding the `:ignore_query => true` option to `assert_current_path` allows the test assertions to ignore any query parameters present in the current URL, providing greater flexibility and robustness when query parameters are appended dynamically.

Actions #3

Updated by Marius BĂLTEANU 15 days ago

  • Description updated (diff)
  • Category changed from My page to Code cleanup/refactoring
Actions #4

Updated by Marius BĂLTEANU 15 days ago

  • Subject changed from Use Capybara's assert_current_path in `log_user` steps to wait for page in ApplicationSystemTestCase to Use Capybara's assert_current_path in "log_user" steps to wait for page in ApplicationSystemTestCase
  • Description updated (diff)
  • Assignee set to Marius BĂLTEANU
  • Target version set to Candidate for next minor release
Actions #5

Updated by Katsuya HIDAKA 9 days ago

The failure of the log_user method test may be related to the following issue reported in Capybara:
Chrome 134 breaks Selenium tests - intermittent failures with visit

In fact, it does not reproduce in my environment (Chrome 131, selenium-driver 4.27.0).

Even so, the patch in #note-2 is a reasonable improvement and it would be good to apply it. I have confirmed that applying the patch in #note-2 resolves the failure of the log_user method test.

Also, the following test failure seems to be affected by the issue reported in Capybara:

Failure:
IssuesSystemTest#test_create_issue_with_attachment [test/test_helper.rb:231]:
"Issue.count" didn't change by 1, but by 0.
Expected: 15
  Actual: 14

bin/rails test test/system/issues_test.rb:134

Actions #6

Updated by Takuya Kodama 7 days ago

Thank you for giving me the above information and confirming my patch.

Actions

Also available in: Atom PDF