Problem/Motivation

PHPUnit tests can fail based on the php.ini settings on different environments. E.g. Memory limit

Proposed resolution

We can set memory limits and error reporting levels in the phpunit.xml.dist configuration file so that tests will always run correctly of different environments like this:

<php>
  <ini name="error_reporting" value="32767"/>
  <ini name="memory_limit" value="-1"/>
</php>

Remaining tasks

Discuss what the correct php ini settings should be and add them phpunit.xml.dist

CommentFileSizeAuthor
#3 2205799_phpunit_config-3.patch584 bytescameron tod
#1 2205799_phpunit_config.patch473 bytesdavidgrayston

Comments

davidgrayston’s picture

davidgrayston commented
StatusFileSize
new 2205799_phpunit_config.patch473 bytes
dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review

Should we maybe document what 32767 means? +1 to set memory limit to -1. We already hit the 100MB limit.

cameron tod’s picture

cameron tod commented
StatusFileSize
new 2205799_phpunit_config-3.patch584 bytes

I think we should! Comments added.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

So I think this change makes sense.

Looking at the PHPUnit error handler code

    public static function handleError($errno, $errstr, $errfile, $errline)
    {
        if (!($errno & error_reporting())) {
            return FALSE;
        }
//...

This will return FALSE if your PHP is configured with error_reporting = E_ALL & ~E_NOTICE and the error number is for a E_NOTICE. Therefore even though PHPUnit is configured with 

         convertErrorsToExceptions="true"
         convertNoticesToExceptions="true"
         convertWarningsToExceptions="true"

by default (http://phpunit.de/manual/3.7/en/appendixes.configuration.html) this actually does not occur.

Setting the error_reporting value in PHPUnit config is a simple fix.

dawehner’s picture

dawehner
Primary language German
commented

Oh cool, E_STRICT is now part of E_ALL.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.