Add support for class_methods do blocks within concerns

[andyw8]

Closes #525

Depends on #529

Here are the new places in Rails we now find:

ActionMailbox::Callbacks
ActionMailbox::Routing
ActionMailer::Rescuable
ActionText::Attachable
ActionText::Attachments::Minification
ActionText::Attachments::TrixConversion
ActionText::Attribute
ActionText::Rendering
ActionText::Serialization
ActionText::Serialization
ActiveRecord::ConnectionAdapters::ColumnMethods
ActiveRecord::Encryption::Configurable
ActiveRecord::Encryption::Contexts
ActiveRecord::Encryption::EncryptableRecord
ActiveRecord::Encryption::ExtendedDeterministicQueries::CoreQueries
ActiveStorage::Attached::Model
Rails::Command::Behavior

[andyw8]

I changed this so that all the code is visible in the tests, rather than having to know what Rails does internally.

[andyw8]

@vinistock I need to do some more manual verification, but opening for any feedback so far.

[andyw8]

Converting to draft until I have a a change to look into the test failures.

[vinistock]

The code looks good to me. Not sure about the test failures, but after a rebase it might be easier to understand what's going on.

[graphite-app]

The comment is inconsistent with the implementation. While it states that reindexing is slow and implies it's being avoided, the code actually reindexes on every test for better isolation. Consider updating the comment to: "Reindex before each test to prevent state leakage between tests, accepting the performance cost for better test isolation."

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

[vinistock]

The issue with the tests is that we're reusing the same names. All examples in this test share the same index instance because the initial indexing takes too long.

Just change the names of the modules and classes involved for each example so that the state doesn't leak between tests.

[andyw8]

@vinistock avoiding the memoization solves in test instability here. It's obviously slower though.

A few possible alternatives:

• Have each test be responsible to removing things it added to the index (maybe compare the state before and after in a teardown to ensure clean-up isn't forgotten)
• Have some naming convention for things we use in tests, and have them automatically removed from the index in a teardown.
• Have a way to skip indexing of RBS in index_all (which takes a large portion of the test time).

[vinistock]

How about we just add note to the test and use different names in each example? Trying to clean up all of the index state might be difficult and it will require exposing some internal APIs.

[vinistock]

Actually scratch that. We already have an API to delete entries, let's use that on teardown.