Add advanced test helpers docs to guides

[sambostock]

Summary

This updates the guides to include examples of useful patterns for organizing test helpers, and preventing test_helpers.rb from getting bloated. It adds two sections:

• Extracting Files, which discusses extracting modules and where they could be put
• Eagerly Requiring Helpers, which discusses how to easily require these modules for convenience

Motivation

I was unable to find good documentation on how to organize test helpers or custom assertions. Unfortunately, the existing documentation's pattern of adding directly to test_helper.rb can lead to that file becoming bloated. The closest related information I could find was RSpec's documentation on the use of spec/support.

Why not test/support?

In my discussion with @rafaelfranca, I had originally leaned towards test/support. However, in the event that the helpers are complex enough to warrant tests, it is not obvious how this should be handled:

• Should the tests be in test/support, and explicitly ignored when eagerly requiring the contents of that directory?
• Should the tests be in another folder, such as test/unit?

@rafaelfranca's suggestion of promoting the helpers to the lib/test directory has an obvious test location of test/lib, making it a good fit for a recommended pattern.

---

• Either update Use ActiveSupport.on_load instead of reopening #34619 after merging this, or update this after merging Use ActiveSupport.on_load instead of reopening #34619

[Edouard-chin]

Use lazy load hook instead

ActiveSupport.on_load(:action_dispatch_integration_test) do
  include SignInHelper
end

[sambostock]

Interesting. I hadn't come across this before. Is there much benefit here, since by this point test_helper has required the Rails environment already.

I can switch to this if it's the preferred pattern, but I basically just copied the code from earlier in the same guide. Should I switch it in both places?

[Edouard-chin]

since by this point test_helper has required the Rails environment already

That's fine, loading the Rails environment doesn't mean the ActionDispatch::IntegrationTest class was loaded.
About benefit; referencing constant too prematurely can lead to some bug that are hard to track down, so better to avoid it.

[gmcgibbon]

I've never seen this used for test code, but I'm not against it. In your test helper, its fairly safe to assume that ActionDispatch::IntegrationTest is loaded, I think.

[Edouard-chin]

https://github.com/rails/rails/blob/5bb4546439ed20a864c6df18788dce0eb45d8326/railties/lib/rails/test_help.rb

[sambostock]

I could open a follow up PR where I update all the places that re-open IntegrationTest to be consistent (in this file, and in the generated test helper).

[Edouard-chin]

Sorry! Forgot to answer 😅 .

I think the generated_test_helper.rb isn't up to date, but anyway using a lazy load hook for ActiveSupport::TestCase or even ActionDispatch::IntegrationTest isn't a big deal compare to using a lazy_load_hook for ActiveRecord or ActionController for example, so I'm fine with either :).

@sambostock Feel free to open another PR to make everything consistent

[gmcgibbon]

Sounds good to me, we can use consistent syntax for now and discuss changing it in another PR.

[rafaelfranca]

Yes, it is a big deal. ActionDispatch::IntegrationTest loads ActionController::Base. For test_helpers the current syntax is fine since this is after the environment is loaded and the application is initialized, but for gems it is a problem.

[sambostock]

Addressed in #34619.

• If this is merged first, I'll update Use ActiveSupport.on_load instead of reopening #34619.
• If Use ActiveSupport.on_load instead of reopening #34619 is merged first, I'll update this.
• If Use ActiveSupport.on_load instead of reopening #34619 is closed without merging, I'll leave this as is.

[gmcgibbon]

I've never seen this used for test code, but I'm not against it. In your test helper, its fairly safe to assume that ActionDispatch::IntegrationTest is loaded, I think.

[gmcgibbon]

👍 Please squash your commits and use [ci skip] to skip Travis