-
-
Notifications
You must be signed in to change notification settings - Fork 984
Migrate SystemTest tests to ApplicationSystemTestCase
#5783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5783 +/- ##
=======================================
Coverage 97.26% 97.26%
=======================================
Files 477 477
Lines 9871 9871
=======================================
Hits 9601 9601
Misses 270 270 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a151945 to
b2b200d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR simplifies the system test suite by removing the custom SystemTest base class (rack-test driven) and migrating remaining tests to Rails’ ApplicationSystemTestCase.
Changes:
- Removed the
SystemTestbase class fromtest/test_helper.rb. - Updated affected system tests to inherit from
ApplicationSystemTestCaseand requireapplication_system_test_case. - Moved
GemsSystemTestout oftest/integration/gems_test.rbinto a dedicatedtest/system/gems_test.rbfile.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/test_helper.rb | Removes the custom SystemTest harness to consolidate on Rails system testing. |
| test/system/yank_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/subscriptions_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/sign_up_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/sign_in_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/search_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/profile_test.rb | Removes redundant test_helper require (now pulled in via application_system_test_case). |
| test/system/policies_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/password_reset_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/pages_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/page_params_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/owner_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/notification_settings_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/locale_test.rb | Migrates to ApplicationSystemTestCase. |
| test/system/gems_test.rb | Adds GemsSystemTest under system tests (previously embedded in integration test file). |
| test/system/email_confirmation_test.rb | Migrates to ApplicationSystemTestCase. |
| test/integration/gems_test.rb | Removes the embedded GemsSystemTest now that it lives under test/system/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Currently, the rubygems.org test suite has 2 different kinds of System tests, one that is using the rails
ApplicationSystemTestCaseand the other, a cut-down version usingrack_testas the driver calledSystemTest. It's hard to tell when we should be using one or the other, and the benefit of keeping it does not seem high either.In an effort to simplify our test suit, i want to remove
SystemTestand migrate everything toApplicationSystemTestCase.