Extracted silence_stream method to new module in activesupport/testing

[vipulnsward]

• Extracted silence_stream method to new module in activesupport/testing
• Added include for the same in ActiveSupport::Test.
• Removed occurrences of silence_stream being used elsewhere.
• Reordered activesupport testcase requires alphabetically.

cc @senny

[carlosantoniodasilva]

I don't think we want to make it publicly available for people to use outside of the framework, that's why it was deprecated and moved to the places it's currently in use. The best thing we can do is to find a way to share code between those places, without making it available through AS::TestCase I think.

[senny]

I agree with @carlosantoniodasilva . My #18381 (comment) was probably not descriptive enough. I think we need to find a new place to share code for our test suites without exposing them through ActiveSupport::TestCase.

[vipulnsward]

I removed the inclusion in ActiveSupport::TestCase and made it available as needed in the tests.

[carlosantoniodasilva]

You can make the method private here, no need to do that from the callers.

[vipulnsward]

Done. Can't move in capture(stream) as it is getting mixed in ActiveSupport::TestCase which defeats our purpose again.
On a side note, I see that these methods that were deprecated from Kernel, are still in guides. Will update guides after this.

[carlosantoniodasilva]

Is it being mixed from as/test_case or in some other abstract unit? I don't think it should be in our test case which would make it available for everyone.

[vipulnsward]

Actually its spread even more- railties abstract unit, railties test isolation, deprecation test, ActiveRecord::TestCase, Rails::Generators::Testing::Behaviour.
Rails::Generators::Testing::Behaviour gets mixed in Rails::Generators::TestCase

[carlosantoniodasilva]

I see.. it was probably mixed in those base classes to make it easier to share, but I think they should never be publicly available (this was the original intent, right?).

Lets move them out as well, people know these are deprecated and being removed anyway.

[rafaelfranca]

Generators tests doesn't work without these methods. They are internal of these test cases, are not public available, just private methods of a public interface.

[carlosantoniodasilva]

Right, so we are good keeping them there. But they should be moved from elsewhere in our suite.

[vipulnsward]

Ok, extracted capture to Stream. Also this shouldn't affect wherever it was getting getting mixed in.

[aditya-kapoor]

It would be also good to focus on those PRs in the recent past where the warnings have been ignored to due non existence of certain instance variables and like. Using this module, we can reduce the unnecessary checks for those variables.
Any thoughts on this?

[carlosantoniodasilva]

Can you show some examples @aditya-kapoor ? In any case I think it's an unrelated discussion.

[carlosantoniodasilva]

Can you add a blank line after the inclusion here?

[vipulnsward]

@carlosantoniodasilva updated.