Make manifest more flexible

[sminnee]

In order to simplify the CMS build, we need to provide Page and Page_Controller classes within the execution of the tests. These need to be picked up by the manifest calls during test execution, but not during the normal manifest processes.

The ideal would be that we could generate a manifest object based on a scan of all files in the project, but then do add "one more class" to it via a run-time call. That way, the extra class could be added in cms/tests/bootstrap.php

As part of this, it may be of value to split out the manifests as a separate package, silverstripe/packagemanifest.

Once this card is done, we can look at simplifying the CMS build.

[tractorcow]

Rather than having a separate package for testing, I'd like to re-raise the idea of recipes that contain project-resources (such as Page.php) for structuring project dependencies. As recipes are no longer root-only, you can simply require these during your test run (composer require silverstripe/recipe-cms) and have your default classes loaded into your project root.

My proof of concept is https://github.com/tractorcow/recipe-cms/blob/master/composer.json.

Having to scan the manifest for classes is exactly the constraint we are trying to get away from by relying more on PSR-4 and composer autoloading; My solution is by no means perfect, but it reduces reliance on install order and improves re-usability (e.g. for CI).

[sminnee]

It's not a separate package for testing. It's about turning manifest into something less brittle and hard-coded.

Providing a recipe or some project files for testing would be one way of continuing to live with a brittle manifest system, but it's not my preferred approach.

[tractorcow]

It's not a separate package for testing. It's about turning manifest into something less brittle and hard-coded.

Ok, I apologise for mis-understanding. However, wouldn't relying on a manifest to bootstrap the test environment still introduce a deviation from testing versus development environment?

[sminnee]

No more so than a test fixture. It's basically about turning the manifest system into something that is able to be manipulated at runtime as a test fixture.

It could possibly be provided into relevant tests rather than bootstrap.php; that can be decided after the core refactoring described in this card is done.

[tractorcow]

I could see this being useful for behat testing as well, should we want to behat test more complex models than what we get out of the box.

[mikenz]

Any reason why you need to keep Page and Page_Controller to run the tests? Our Page class is sufficiently customized that a lot of the tests don't run so we've moved most the unit tests to use SiteTree or test specific classes that extent SiteTree. This makes the tests independant of the customization done on the installation. eg

git diff -w github/master  tests/controller/CMSMainTest.php
diff --git a/tests/controller/CMSMainTest.php b/tests/controller/CMSMainTest.php
index ceafabe..045a0eb 100644
--- a/tests/controller/CMSMainTest.php
+++ b/tests/controller/CMSMainTest.php
@@ -58,7 +55,7 @@ class CMSMainTest extends FunctionalTest {
         $hints = Convert::json2array($rawHints);

         $this->assertArrayHasKey('Root', $hints);
-        $this->assertArrayHasKey('Page', $hints);
+        $this->assertArrayHasKey('CMSMainTest_Page', $hints);
         $this->assertArrayHasKey('All', $hints);

         $this->assertArrayHasKey(
@@ -103,7 +100,7 @@ class CMSMainTest extends FunctionalTest {

         // Page A can't have unrelated children
         $this->assertContains(
-                'Page',
+                'CMSMainTest_Page',
                 $children,
                 'Limited parent lists disallowed classes'
         );
......

@@ -606,10 +616,13 @@ class CMSMainTest_ClassB extends Page implements TestOnly {

 }

-class CMSMainTest_NotRoot extends Page implements TestOnly {
+class CMSMainTest_NotRoot extends SiteTree implements TestOnly {
     private static $can_be_root = false;
 }

-class CMSMainTest_HiddenClass extends Page implements TestOnly, HiddenClass {
+class CMSMainTest_HiddenClass extends SiteTree implements TestOnly, HiddenClass {
+
+}

+class CMSMainTest_Page extends SiteTree implements TestOnly {
 }

[sminnee]

Any reason why you need to keep Page and Page_Controller to run the tests?

You couldn't test ErrorPage without it. Or any other page type provided by a module that extends Page.

Subclasses of Page provided by modules are a pattern that would be good remove, but that's more of an SS5 problem.

[mikenz]

You couldn't test ErrorPage without it. Or any other page type provided by a module that extends Page.

Good enough reason.

[sminnee]

Yeah, that said, there's no reason why the core tests can't be modified in the way you propose—it makes for less brittle tests. It's just not a full solution to the problem I've come across.

The use case is to support the running of CMS tests without an actual SilverStripe project:

git clone https://github.com/silverstripe/silverstripe-cms.git
cd silverstripe-cms
composer install
vendor/bin/phpunit

It's a bit unusual compared to the last 10 years of SilverStripe's history, but much more in keeping with the rest of the composer ecosystem.

[sminnee]

ModuleManifest already exists as a class. As long as we retain the existing functionality, we can add more search locations for modules as a minor change.