FIX Add reports and siteconfig tests

[dhensby]

No description provided.

[mention-bot]

By analyzing the blame information on this pull request, we identified @sminnee, @chillu and @halkyon to be potential reviewers

[tractorcow]

Rebase please.

[sminnee]

This file is the default phpunit.xml.dist file provided for new SilverStripe projects. I'm not convinced that running the tests of all our supplied modules is the best default set-up for new projects. It's time-consuming and out of keeping with the practises of other projects/ecosystems.

I think that this should only contain mysite/tests and in so doing let users test their own projects by default.

I assume that our own test runs ignore the paths specified in this file? If not, can we please have a separate file, such as phpunit.xml.travis, that our tests use.

[dhensby]

This change was "needed" to make sure https://github.com/silverstripe/cow tested all the modules when doing a release.

The is the dist file meant to be more of an example or something for devs to rely upon? I'd expect a dev to create their own non-dist version...

[sminnee]

In that case, cow is wrong and should be fixed. If we're making life hard for our users to make it easier for a cow, we've got it backward.

phpunit.xml.dist is used as the default config file for phpunit and should specify the test configuration for the project in question, which in this case is the newly minted SilverStripe site created by the create-project command, or the unpacking of a SilverStripe zip or tar.gz.

[dhensby]

phpunit.xml.dist is used as the default config file for phpunit and should specify the test configuration for the project in question, which in this case is the newly minted SilverStripe site created by the create-project command, or the unpacking of a SilverStripe zip or tar.gz.

Just for my own clarification: you're saying that you don't think the test suites of the included modules should be part of this configuration for a new project?

[sminnee]

Just for my own clarification: you're saying that you don't think the test suites of the included modules should be part of this configuration for a new project?

Yes, that's what I'm saying. Admittedly I have seen a lot of people (including myself and other members of the core team) have set up project builds to run all module tests in the past, but I think this is a bad idea for the following reasons:

• It's slower. It makes TDD harder and overwhelms CI servers. It will encourage SilverStripe developers to think of TDD and automated testing as something slow and hard.
• It's out of line with the norms of other projects. When you install Symfony, you don't, by default, run the Symfony test suite as part of your project.
• Most failures that it highlights are false-positives, caused by global-state pollution.
• If shifts responsibility for maintaining core features to the project developer.

I can see a few areas where we would want to include module tests in the project builds:

• If project developer make their own modules that aren't otherwise tested, then they could be included. However, this is easily resolved by the module creator updating phpunit.xml.dist themselves.
• If we supplied some kind of "generic sanity check test suite", that, for example, confirmed that no page type returns a 500 error, that would be appropriate to run as part of project builds. This would be a cool idea, but I don't think this exists.

[dhensby]

OK - well, I'm happy with trimming down the dist file, but maybe that's a master change?

I think seeing as how we already test most of the core suite, we should probably add these to make it consistent with the rest of 3 and then strip it out for 4.

However, I'm not too bothered - feed free to close or merge depending on preference!

[tractorcow]

Could we still have a separate phpunit.xml that runs core tests, but isn't included by default by nature of a different file name? phpunit.core.xml? That way you could still specify it with the -c option when running phpunit.

The default phpunit.xml should still point to user-tests.

[dhensby]

Again, happy for someone to close this if that's preferred.

Given 3.x distributes with testing the included modules, I think this should go into 3 and then we remove all from master - but, again, not that fussed

[chillu]

I think there's limited value in a webroot-level phpunit.xml.dist which tests all modules. Sure, it's handy for contributors who want to regression test their fix across all core modules, but I don't think that's happening in reality. If you're lucky, contributors run vendor/bin/phpunit framework/tests, but not vendor/bin/phpunit. Having phpunit.xml.dist only test mysite/tests is also my preference. It's important to keep the file around in the first place, because regardless which module you're testing, you'll need <phpunit bootstrap="framework/tests/bootstrap.php" colors="true">. So removing it altogether from the installer will make it harder for users to get started on their own tests.

So +1 for closing this PR, and removing these entries:

<directory>cms/tests</directory>
<directory>framework/tests</directory>
<directory>framework/admin/tests</directory>

-1 against removing phpunit.xml.dist from installer altogether.

[tractorcow]

If we do this, what's the best way for us to deterministically run all core tests during release? a .cow/phpunit.xml? Or should we explicitly search all <module>/test dirs and run them one by one?

[tractorcow]

I've raised a placeholder issue at silverstripe/cow#13

[sminnee]

I don't think it should so much be a specific phpunit.xml file as specific pre-build test command. That will let us cover, for example, javascript tests if we wished to.

I would have a look into whether custom composer.json extras or scripts entries might be useful for this...