[openstreetmap/openstreetmap-website] WIP: Use rack by default for system tests (PR #6878)

[Andy Allan]

This speeds up the system tests whereever a full-blown JS engine is not required. When it is, we can either use `js_test` to mark one specific test, or `driven_by_selenium` to make the whole file us the full browser.

This is a variation on #6497. It switches to use rack by default, and then a custom `js_test` to run tests with selenium (i.e. using a full browser) when necessary. It also has a class-level `driven_by_selenium` for when the whole file needs javascript.

Things to consider:
* In the files with mixed use of `test` and `js_test`, you need to run any `sign_in_as` within the `js_test` block. Otherwise you log in using the default `rack_test` driver and then the selenium driver isn't logged in.
* I'm not convinced that the `driven_by_selenium` option is actually a good idea, but I'm interested in feedback. It means that some js-is-required tests use `js_test` but others use `test` (when there's a class-level `driven_by_selenium`). I think it might be more confusing to do it this way, compared to just using `js_test` throughout.
* If we keep the class-level statement, I would be open to renaming it to something like `all_tests_need_js` or somesuch.
* I measured about a 10%-15% speedup across the system tests with this approach, because many tests still need javascript. This is for a variety of reasons, some of which could be refactored (e.g. using positional `:before/:after` in assertions). But are the overall savings worth the effort?

Overall though, I think it's an improvement on the split-classes (or potential metaprogramming) approach in #6497

Feedback welcome.
You can view, comment on, or merge this pull request online at:

  https://github.com/openstreetmap/openstreetmap-website/pull/6878

-- Commit Summary --

  * Use rack by default for system tests

-- File Changes --

    M test/application_system_test_case.rb (44)
    M test/system/account_deletion_test.rb (9)
    M test/system/account_home_test.rb (2)
    M test/system/browse_comment_links_test.rb (2)
    M test/system/changeset_comments_test.rb (10)
    M test/system/changeset_elements_test.rb (2)
    M test/system/create_note_test.rb (2)
    M test/system/dashboard_test.rb (6)
    M test/system/diary_entry_test.rb (2)
    M test/system/directions_test.rb (2)
    M test/system/element_current_version_test.rb (4)
    M test/system/element_history_test.rb (6)
    M test/system/embed_test.rb (2)
    M test/system/history_test.rb (2)
    M test/system/index_test.rb (2)
    M test/system/issues_test.rb (2)
    M test/system/note_comments_test.rb (8)
    M test/system/note_layer_test.rb (2)
    M test/system/oauth2_test.rb (2)
    M test/system/profile_image_change_test.rb (4)
    M test/system/profile_links_change_test.rb (10)
    M test/system/query_features_test.rb (2)
    M test/system/redaction_destroy_test.rb (2)
    M test/system/resolve_note_test.rb (2)
    M test/system/search_test.rb (8)
    M test/system/select_language_test.rb (2)
    M test/system/site_test.rb (2)
    M test/system/user_login_test.rb (2)
    M test/system/user_logout_test.rb (22)
    M test/system/user_status_change_test.rb (2)
    M test/system/users_test.rb (2)

-- Patch Links --

https://github.com/openstreetmap/openstreetmap-website/pull/6878.patch
https://github.com/openstreetmap/openstreetmap-website/pull/6878.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6878
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/6878 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260311/45d2f695/attachment-0001.htm>