govCMS / govcms8-scaffold-paas

GovCMS8 PaaS Project Scaffolding
5 stars 7 forks source link

Effective unit and functional test delineation for phpunit #14

Open simesy opened 5 years ago

simesy commented 5 years ago

In phpunit configuration we see

    <testsuite name="unit">
      <file>/app/web/core/tests/TestSuites/UnitTestSuite.php</file>
    </testsuite>
    <testsuite name="kernel">
      <file>/app/web/core/tests/TestSuites/KernelTestSuite.php</file>
    </testsuite>
    <testsuite name="functional">
      <file>/app/web/core/tests/TestSuites/FunctionalTestSuite.php</file>
    </testsuite>
    <testsuite name="functional-javascript">
      <file>/app/web/core/tests/TestSuites/FunctionalJavascriptTestSuite.php</file>
    </testsuite>

We don't need core testsuites, that is outside scope of the project. We also need to delineate between non-bootstrapped and bootstrapped tests.

Remember that for a project that is not using behat properly, phpunit functional (bootstrapped) tests are recommended, but this is a very different setup in CI compared to static tests.

Something that is client-oriented might look like this:

<testsuite name="unit">
     <directory>./web/modules/custom/*/tests/src/Unit</directory>
</testsuite>

As speculation for pure static unit tests - ie testing a class with everything injected and mocked - there are a few generic places we might expect these to live.

  1. ./tests/phpunit/unit or ./tests/phpunit/tests/unit (tbc)
  2. ./web/modules/custom/*/tests/src/Unit
  3. ./web/themes/custom/*/tests/src/Unit (entirely plausible on saas)

I think this is possible as a sane default, but I need to check syntax.

<testsuite name="unit">
     <directory>./tests/phpunit/Unit</directory>
     <directory>./web/modules/custom/*/tests/src/Unit</directory>
     <directory>./web/themes/custom/*/tests/src/Unit</directory>
</testsuite>
simesy commented 5 years ago

This is good on one hand, but it doesn't separate unit/functional tests. We could use --group=theme if that's possible.

<testsuite name="theme">
      <directory>/app/web/themes</directory>
</testsuite>

path should be /app/web/themes/custom too...

steveworley commented 5 years ago

Core documents how contributed/custom modules should write phpunit tests. This is compatible with the default phpunit.xml configuration. The configuration that we're using is the base configuration file with a suite added for our customisations.

I vaguely remember being able to use groups with the default configuration to run tests from specific modules (albeit we'd need multiple groups), so it would end up looking something like:

phpunit --group my_custom_mdoule, my_custom_theme

Perhaps we can look to leverage groups a little more and define some standard groups that will allow us to continue using the Core provided configuration.

I think we will want to differentiate between kernel and unit tests, Drupal provides test suites for that already, Kernel gives us full stack (including DB connections) and unit is more limited aimed at just code testing.

So if we look for a standard group we could invoke phpunit like

phpunit --testsuite unit --group govcms
simesy commented 5 years ago

phpunit --testsuite unit --group govcms

Ok. So to do this I think we'd have to:

  1. widen the scope of the unit testsuite to include custom directories, and
  2. developers would annotate their tests with govcms

Kernel gives us full stack (including DB connections)

Yeah that includes functional/integration tests.

AlexSkrypnyk commented 5 years ago

I think the solution should be agnostic to any groups for clients:

vendor/bin/phpunit -c /app/docroot/core/phpunit.xml.dist docroot/modules/custom/ --filter '/.*Unit.*/' 
vendor/bin/phpunit -c /app/docroot/core/phpunit.xml.dist docroot/modules/custom/ --filter '/.*Kernel.*/
vendor/bin/phpunit -c /app/docroot/core/phpunit.xml.dist docroot/modules/custom/ --filter '/.*Functional.*/

This is because any type of grouping is not reliable - developers may write a test and forget to tag it with the group.

Also, note that /app/docroot/core/phpunit.xml.dist is a Core's phpunit config file, which is very important - there should not be any alterations to how tests run or potentially the requriements for test runs may be softened etc. Or at least, this should be configurable as a variable and defaulted to Core's phpunit.xml.

This approach is already successfully working on several projects.

simesy commented 4 years ago

fwiw, I disagree agree with using core's /app/docroot/core/phpunit.xml.dist file because it's out of our control and a different use case.

AlexSkrypnyk commented 4 years ago

ok, looks like changing the contents of the config file + using groups should be an accepted solution

<testsuite name="unit">
     <directory>/app/tests/phpunit/Unit</directory>
     <directory>/app/web/modules/custom/*/tests/src/Unit</directory>
     <directory>/app/web/themes/custom/*/tests/src/Unit</directory>
</testsuite>

This is now ready to be submitted as a PR.

simesy commented 4 years ago

I love that approach