Missing assertions #5002
Missing assertions #5002
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5002 +/- ##
=======================================
Coverage 97.02% 97.02%
=======================================
Files 420 420
Lines 8726 8726
=======================================
Hits 8466 8466
Misses 260 260 ☔ View full report in Codecov by Sentry. |
simi
force-pushed
the
missing-assertions
branch
from
caa5c1c
to
155df7e
Compare
| @@ -223,7 +223,7 @@ class ApiKeysControllerTest < ActionController::TestCase | |||
| end | |||
|
|
|||
| should "show error to user" do | |||
| page.assert_text "Show dashboard scope must be enabled exclusively" | |||
| assert page.has_content? "Show dashboard scope must be enabled exclusively" | |||
There was a problem hiding this comment.
these changes actively make assertion failure worse, can we migrate at least to the capybara minitest methods to maintain the good error messages?
There was a problem hiding this comment.
That's the case I have explained in the description. It is not possible to simply including capybara assertions since they do conflict with rails-dom-testing gem. For example assert_select does different kind of assertion. Happy to move those tests to be integration ones in next PR and remove rails-dom-testing dependency (maybe rails-controller-testing in general) at all.
There was a problem hiding this comment.
Alternatively I can manually port assert_content assertion to functional tests to cover most of the similar changes to report with nice and friendly message before mentioned refactoring.
One step closer to warning-less Rails 7.2. I was pretty skeptic to this new feature first, but it helped to reveal few broken places doing no assertions and testing nothing. Also it reveals quite strange mix of some test responsibilities like functional tests testing template outputs using Capybara (that's what system/integration test should do IMHO) and vice versa. I did the smallest change for now to eliminate all warnings.