backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Change the default profile on tests from Standard to Testing #4943

Open hosef opened 3 years ago

hosef commented 3 years ago

Description of the need

Currently most tests are using the Standard profile. This causes lots of unnecessary overhead on each test because most tests only need a few modules installed.

Proposed solution

Change the default profile to one that has fewer tables to copy when setting up a new test prefix.

Additional information

This will probably break tests in contrib. This should be easy to mitigate by setting protected $profile = 'standard'; if you depend on something from the standard profile, but it will require changes from module maintainers.

indigoxela commented 3 years ago

This will probably break tests in contrib.

And that's a good reason to not change the default. :wink:

I support the main intention behind this issue, but my proposal would be to change the profile - one by one - in core tests. I assume, there will be a lot of work fixing these broken tests, but the effort will be more predictable. It's not that we have a lot of experts for writing/fixing tests, neither in the core team, nor in contrib.

@hosef is there a way to identify (core) tests that would be significantly faster, when switching their profile? Or is this just a general test performance improvement? (Is there some estimation about the actual test performance improvement?)

klonos commented 3 years ago

I also understand the intention of this, but most actual Backdrop sites will be using the Standard profile. So in my head, I think that running the tests against the Standard profile will help catch errors/fails in real-life use cases. The Testing profile, although slim and fast, may lack things/config/content that we need in order to catch (more) failures. ...I could be totally wrong about this though 🤷

hosef commented 3 years ago

@indigoxela One example which is pretty extreme is ViewsHandlerFilterStringTest which sets up new test prefixes 25 times. On my local environment, it's time is reduced from about 35 seconds to about 25 seconds. As far as I can tell from doing performance testing a large variety of test classes on my local environment, almost 50% of test execution time is spent setting up prefixes, and almost 40% of the time is spent clearing caches after transferring into the test prefix.

@klonos StandardInstallTestCase is supposed to make sure that the standard profile is working, although it is lacking quite a bit in that regard. The point of the other tests it to confirm that Backdrop's various components work in isolation from each other so that we can find problems more quickly. If the system is completely dependent on a particular profile, then we effectively don't support installation profiles.

indigoxela commented 3 years ago

One example which is pretty extreme is ViewsHandlerFilterStringTest

Wow, yeah, that's extreme...

On my dev VM (single core CPU, known to be slow with those tests) I ran:

sudo -u www-data /usr/bin/php7.4 ./core/scripts/run-tests.sh --url http://bla.tld ViewsHandlerFilterStringTest

on a vanilla install (1.18.1): Test run duration: 17 min 28 sec

Then I added a single line of code in core/modules/views/tests/handlers/views_handler_filter_string.test: protected $profile = 'testing'; and ran the exact same command again (note: no caching in both runs):

Test run duration: 5 min 5 sec

If that isn't significant, what could ever be? :laughing:

I'd still recommend to not change the default, but change the profile per test - it's a one-liner per test and it does no harm to other tests.