Do not mark classes that inherit from ActiveJob::TestCase as dead

[nvasilevski]

In #579 we stopped ignoring class names that end with Test which caused test classes that inherit from ActiveJob::TestCase to be indexed as dead. It's not an issue for classes inherited from ActiveSupport::TestCase because we have a plugin that ignores those:

spoom/lib/spoom/deadcode/plugins/active_support.rb

Line 8 in 3e8ae20

ignore_classes_inheriting_from("ActiveSupport::TestCase")

We should do the same for ActiveJob::TestCase. Since we already have ActiveJob plugin I add an ignore macro to the plugin.

[Morriar]

Isn't ActiveJob::TestCase a subclass of ActiveSupport::TestCase (https://github.com/rails/rails/blob/main/activejob/lib/active_job/test_case.rb#L6)? The ActiveSupport plugin should be catching this so there is another kind of bug at play here. I don't think this is the right fix.

[Morriar]

Oh I think I know what is happening here. We only index .rb files in CodeDB: https://github.com/Shopify/code-db/blob/main/app/jobs/jobs/project/spoom_deadcode.rb#L9. The relationship between ActiveJob::TestCase and ActiveSupport::TestCase may only exist in the RBI files so the Spoom Model doesn't see it...

I wonder what would be the impact of also adding .rbi files to the Model (not the dead code Index though as we don't want to report anything about RBI files). We should time it to see how much slower it makes the analysis.

[nvasilevski]

Isn't ActiveJob::TestCase a subclass of ActiveSupport::TestCase

It definitely is but since it wasn't working I just assumed that "for reasons" we only look at the direct parent without trying to analyze the whole inheritance chain.

But if we expect ignore_classes_inheriting_from to apply to the whole chain then I totally agree we should be looking into keeping only ActiveSupport::TestCase as ignored parent and fixing the lookup instead

I wonder what would be the impact of also adding .rbi files to the Model

I can allocate some time to do that but since we are experiencing an issue right now and current version has a bug how would you feel about either merging this PR with explicit ignore of ActiveJob::TestCase or perhaps temporarily removing changes to Minitest plugin (bringing back ignore of *Test classes

[Morriar]

I think we can merge this. Though, since you know Rails can you ensure that we not missing other subclasses of ActiveSupport::TestCase?

I'll handle the RBI indexing exploration 👍

[nvasilevski]

Since ActiveJob is not the only framework with its own subclass we are going to temporarily bring back the exclusion of *Test classes until we find a proper solution which will allow only ActiveSupport parent to be ignored