Don't update tracker js files while testing #17155
Conversation
sgiehl
added
c: Tests & QA
|
btw be good to check if this actually speeds up the tests a lot. If it's just a minute or less then it might not be worth it merging as we had issues with it in the past and it can be good to have the tests cover this logic. Eg when a plugin gets activated, and adds tracking code or does something else with tracking code, and then reads a DI config as part of this but because the plugin was earlier not loaded the DI config was not read then it fails. Generally we'd want to to run this code. Or if the file system is slow to write the changes then we could simply store it in memory or so instead of writing it to disk (but then we'd probably also need to read from memory and then things get complicated and might not be worth it). |
There was a problem hiding this comment.
@sgiehl do you know how much of a time difference that makes? I'm not 100% sure but I remember some cases where I had found bugs/issues by having these events executed in the tests eg when some dependencies were missing in other plugins that influence the tracker generation etc. If we're not saving too much time it might make sense to keep the logic there or to maybe rather replace file handles with in memory files but whether it's worth the effort would depend on how much time this saves. Generally we want to aim to execute as much of the actual logic during tests as possible.
|
This doesn't seem to have an effect for any test suite, except the first system test build: 4.x-dev: this pr: (I restarted the 4.x-dev build so it's repeatable.) This suggests some or one of the core system tests needs this while it's ok to run in other tests. Maybe we can find where specifically it's causing a big problem? Assuming it's better to run as much code as possible during tests. |
|
@diosmosis I'll simply close this one. Guess the long run time on 4.x-dev was randomly. |
Description:
The
CustomTrackerJSplugin is enabled by default and tries to update the tracker js files whenever certain events are triggered. Those events include the creation of a site and other events that are triggered a lot while testing.This PR disables the event listening for that plugin while testing, and only enables it for those tests where it is actually needed.
This should speed up the tests as it also avoids a lot file operations.
Review