Assert status checks pass as early as possible in kernel and functional tests

💯, this will likely allow me to close that other issue at this point! 🎉

(Typing this from my phone, will find it later.)

As promised in #6.

OK, so! I spent an hour going over this today with @yash.rode, so I could understand the problem thoroughly. And now I do.

I see why this has been so knotty to untangle - it's hard to get in and do the alterations we need to do, at the time we need to do them. But, I have pushed a solution that is related, but slightly different. I'd much prefer we DID NOT override setUpFilesystem() for all Package Manager kernel tests -- changing something that low-level, that broadly, is asking for trouble. Especially when the problem is really arising from a surgical problem in the way that SiteFilesExcluder interacts with UnknownPathExcluderTest.

Therefore, I suggest we do the following:

Split UnknownFilesExcluderTest into two tests: UnknownFilesExcluderTest, which doesn't have a relocated docroot (or, if you prefer, "nested web root"), and UnknownFilesExcluderWithRelocatedDocrootTest extends UnknownFilesExcluderTest which DOES have a relocated docroot. It should set that up simply by overriding createTestProject() and moving the files around, much as is already done in setUpFilesystem(). Then, the data provider in UnknownFilesExcluderTest can be slimmed down; it only needs four cases, and the same cases will be tested in a relocated docroot by the subclass.

I know what's breaking these tests, so self-assigning to resolve those.

Therefore, I suggest we do the following:

Split UnknownFilesExcluderTest into two tests: […]

Then, the data provider in UnknownFilesExcluderTest can be slimmed down; it only needs four cases, and the same cases will be tested in a relocated docroot by the subclass.

That's exactly what I suggested and what I was asking @yash.rode to do 😅

But doing it in createTestProject() is too late based on my analysis late last week (when I first paired for a solid while with @yash.rode to get him on track to fixing this), because the kernel will already have booted at that point, and that then makes excluders think the project root is somewhere else than it actually is.

Looking forward to seeing my understanding getting proven wrong though! 😄

I think this will pass, so assigning to Wim for review.

phenaproxima → closed merge request !771

Given some of the clean-up I've been able to do here, I'm concerned about UnknownFilesExcluder. Either it isn't working, or our coverage is fatally flawed. Need to examine more closely...

I think I need to take a closer look at this and refactor that test to be...like, understandable. It's WAY too complicated right now to make head or tail out of. That, for the record, is a pre-existing condition in HEAD.

That said, this is post-MVP, so I have to deprioritize it for now. I'm leaving this assigned to myself, since I have my head wrapped around it pretty clearly.

I think 90% of the complexity here could be removed if we would execute $this->assertStatusCheckResults([]); in a different kernel. But that too is non-trivial. But at least it would avoid changing code all over the place just for the sake of avoiding static caching that is otherwise totally reasonable.

This was opened at a time where we had lots of productivity/velocity issues due to absence of validators, less strict validators, and we especially did not yet have the concept of base requirement validators (introduced in 📌 Create an API for base requirement validators which run before other validators and stop event propagation if they fail Fixed ).

Package Manager & Automatic updates are wildly different today compared to 3.5 months ago:

$ git log --oneline --since 2023-01-23 | wc -l
     171

git d b485deb6 414dd683
…
 344 files changed, 12984 insertions(+), 10245 deletions(-)

$ git d b485deb6 414dd683 -- package_manager/src/Validator
 package_manager/src/Validator/AllowedScaffoldPackagesValidator.php    |  80 ++++++++++++++++++++++
 package_manager/src/Validator/BaseRequirementValidatorTrait.php       |  50 ++++++++++++++
 package_manager/src/Validator/BaseRequirementsFulfilledValidator.php  |  71 +++++++++++++++++++
 package_manager/src/Validator/ComposerExecutableValidator.php         | 208 -------------------------------------------------------
 package_manager/src/Validator/ComposerJsonExistsValidator.php         |  75 --------------------
 package_manager/src/Validator/ComposerMinimumStabilityValidator.php   |  90 ++++++++++++++++++++++++
 package_manager/src/Validator/ComposerPatchesValidator.php            | 165 +++++++++++++++++++++++++++++++++++++++-----
 package_manager/src/Validator/ComposerPluginsValidator.php            | 240 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 package_manager/src/Validator/ComposerSettingsValidator.php           |  66 ------------------
 package_manager/src/Validator/ComposerValidator.php                   | 143 ++++++++++++++++++++++++++++++++++++++
 package_manager/src/Validator/DiskSpaceValidator.php                  |  35 ++--------
 package_manager/src/Validator/DuplicateInfoFileValidator.php          |  84 +++++++++--------------
 package_manager/src/Validator/EnabledExtensionsValidator.php          |  88 ++++++++++++++++++++++++
 package_manager/src/Validator/EnvironmentSupportValidator.php         |  28 ++++----
 package_manager/src/Validator/LockFileValidator.php                   |  70 ++++++++-----------
 package_manager/src/Validator/MultisiteValidator.php                  |  47 ++++---------
 package_manager/src/Validator/OverwriteExistingPackagesValidator.php  |  77 ++++++++-------------
 package_manager/src/Validator/PendingUpdatesValidator.php             |  40 +++--------
 package_manager/src/Validator/PhpExtensionsValidator.php              | 103 ++++++++++++++++++++++++++++
 package_manager/src/Validator/PhpTufValidator.php                     | 180 ++++++++++++++++++++++++++++++++++++++++++++++++
 package_manager/src/Validator/PreOperationStageValidatorInterface.php |  25 -------
 package_manager/src/Validator/SettingsValidator.php                   |  21 ++----
 package_manager/src/Validator/StageNotInActiveValidator.php           |  34 ++++-----
 package_manager/src/Validator/StagedDBUpdateValidator.php             |  67 ++++++------------
 package_manager/src/Validator/SupportedReleaseValidator.php           |  36 +++++++---
 package_manager/src/Validator/SymlinkValidator.php                    | 122 ++++++++++-----------------------
 package_manager/src/Validator/WritableFileSystemValidator.php         |  40 ++---------
 package_manager/src/Validator/XdebugValidator.php                     |  85 -----------------------
 28 files changed, 1431 insertions(+), 939 deletions(-)

and perhaps most crucially:

 package_manager/tests/src/Kernel/AllowedScaffoldPackagesValidatorTest.php   |  61 +++++++++
 package_manager/tests/src/Kernel/BaseRequirementsFulfilledValidatorTest.php |  93 ++++++++++++++
 package_manager/tests/src/Kernel/ComposerMinimumStabilityValidatorTest.php  |  54 ++++++++
 package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php           | 304 ++++++++++++++++++++++++++++++++++++-------
 package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php           | 405 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 package_manager/tests/src/Kernel/ComposerValidatorTest.php                  | 173 +++++++++++++++++++++++++
 package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php                 |  11 +-
 package_manager/tests/src/Kernel/DuplicateInfoFileValidatorTest.php         |  64 +++++-----
 package_manager/tests/src/Kernel/EnabledExtensionsValidatorTest.php         | 163 +++++++++++++++++++++++
 package_manager/tests/src/Kernel/EnvironmentSupportValidatorTest.php        |  10 +-
 package_manager/tests/src/Kernel/LockFileValidatorTest.php                  |  69 +++++++---
 package_manager/tests/src/Kernel/MultisiteValidatorTest.php                 |  69 ++++++----
 package_manager/tests/src/Kernel/OverwriteExistingPackagesValidatorTest.php | 130 +++++++++++--------
 package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php            |   8 +-
 package_manager/tests/src/Kernel/PhpExtensionsValidatorTest.php             |  77 +++++++++++
 package_manager/tests/src/Kernel/PhpTufValidatorTest.php                    | 250 ++++++++++++++++++++++++++++++++++++
 package_manager/tests/src/Kernel/StagedDBUpdateValidatorTest.php            |   7 +-
 package_manager/tests/src/Kernel/SupportedReleaseValidatorTest.php          |  36 ++----
 package_manager/tests/src/Kernel/SymlinkValidatorTest.php                   | 277 +++++++++++++++++++++++----------------
 package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php        |  11 +-
 20 files changed, 1949 insertions(+), 323 deletions(-)

So … I'm going to mark this .

I agree this is outdated. Thanks for the work on it though