Remove caching of test classes in TestDiscovery

I guess that's a left-over of the times when Simplenews had to load and parse all .test files which was possibly very slow.

The thing is, we're not going back to parsing PHP files, but we are going to back to recursive directory scans of possibly hundreds (a real site with contrib modules) of directories. I'm not sure I agree with removing that cache without a performance test.

I spent some time verifying the UI display of skipped tests for #2296615: Accurately support multiple @groups per test class. The workflow was: Show the list of tests, clear cache, reload, clear cache, reload...

By far the slowest part of the simpletest UI form is the rendering, not the file scan.

run-tests.sh uses the same test discovery code, but it only performs the scan once, and really shouldn't be using a cache.

This patch just removes references to the cache from TestDiscovery.

Still to do:

• Adjust test_discovery service definition so it doesn't need the file cache service.
• Ensure that no part of core passes the cache service in.

Yes, I also noticed the fact that rendering that table takes far more time than discovery.

Do you have any specific numbers on how long discovery vs rendering takes?

We could relatively easily add render caching for the table (we could even make it so that we use a pre_render callback and only call the discovery on a cache miss) but we have no reliable way for invalidation, maybe we could just keep the button and that invalidates a custom cache tag that we use on that table? The result is that we only have caching in the UI and caching that actually helps a lot more than what we have right now.

Web Profiler reports that admin/config/development/testing takes about 25 seconds TTFB.

Added is a patch that runs TestDiscovery a bunch of times to compare cached and uncached performance.

Setting $iterations to 1, the as-is cached getTestClasses() takes about 2.2 seconds. Uncached is about 1.8, which seems like a lot of overhead for managing a cache, so maybe my numbers are bad. Obviously this will vary with machine and OS and all kinds of other mysterious factors.

Anyway, if you advance it to 2 iterations it's obvious that you want the cache. :-) Two cached calls takes 2.2 seconds, uncached is 3.5.

So if we're never calling it more than once we want to kill the cache, but if we need to call it multiple times per request, then we like the cache.

However, we don't like the cache due to DX and UX, because as mentioned the cache persists too well and we have to figure out that we need to clear it, and then clear it in order to recognize a new test.

So we have two options:

1. Decide that we will never call TestDiscovery->getTestClasses() more than once or else live with the consequences.
2. Give SimpletestTestForm a way to discover tests without the cache. The patch below does this by invalidating the cache before TestDiscovery can use it.

Also: TestDiscovery::getTestClasses() instantiates a SimpleAnnotationReader but then never uses it.

Updating IS, linking to #2863055: Move TestDiscovery out of simpletest module, minimize dependencies

Do we have a follow up with the work @Berdir suggested which is caching the actual HTML of that page?

Behold: #2893117: Improve HTML caching of Simpletest UI test form

Soft-blocker for this issue could use some attention: #2893117: Improve HTML caching of Simpletest UI test form

The solution in #2893117-21: Improve HTML caching of Simpletest UI test form removes the cache from TestDiscovery, so this issue might be redundant.

#2893117: Improve HTML caching of Simpletest UI test form is in, and it removes persistent caching of discovered tests. It keeps an internal cache of discovered tests per instance, to avoid repeated file system scans per request.

That makes this issue out of date.

This is re-opened because #2893117: Improve HTML caching of Simpletest UI test form was reverted, but postponed because that issue is really the solution.

And now we have #2893117: Improve HTML caching of Simpletest UI test form back again, which still makes this issue outdated by removing the caching of test classes from TestDiscovery, other than a per-request cache.