Fix some more ignored block warnings #51608
Conversation
Ref: https://bugs.ruby-lang.org/issues/15554 A couple are harmless, but another couple found actual problems in the test suite where we passed blocks to `assert_*` methods that didn't expect one.
casperisfine
force-pushed
the
block-warning-fixes
branch
from
72f16ef
to
d5c88d6
Compare
| @@ -502,12 +502,12 @@ def test_decorated_type_with_decorator_block | |||
| klass = Class.new(ActiveRecord::Base) do | |||
| self.table_name = Topic.table_name | |||
| store :content, coder: ActiveRecord::Coders::JSON | |||
| attribute(:content) { |subtype| EncryptedType.new(subtype: subtype) } | |||
| attribute(:content, EncryptedType.new(subtype: Topic.attribute_types.fetch("content"))) | |||
There was a problem hiding this comment.
Is this required change? Actually this test case tests attribute with a decorator block in accordance with its test name.
There was a problem hiding this comment.
Is this required change?
The attribute method doesn't take a block and never yields. I'll go check if it did when this test was introduced, but I don't think it ever did. So this test was essentially a noop.
There was a problem hiding this comment.
Alright, this was changed in #44666.
So I think:
- This test should use
decorate_attributesinstead. attributeshould warn or raise when given a block, because that same issue may be present in users code.
There was a problem hiding this comment.
This internal feature and test was introduced in #41139 by me.
There was a problem hiding this comment.
Right, but as of #44666 it no longer exist. If it was never public API, then I suppose we don't need to warn or raise.
I updated the test to use the new API.
There was a problem hiding this comment.
Yeah, I didn't know that had been replaced to decorate_attributes, thanks for the quick investigation!
There was a problem hiding this comment.
I updated the test to use the new API.
Somehow forgot to push... I'll open another PR.
Ref: https://bugs.ruby-lang.org/issues/15554
A couple are harmless, but another couple found actual problems in the test suite where we passed blocks to
assert_*methods that didn't expect one.