Skip to content

Silent failure when configuration file is invalid #6590

@theofidry

Description

@theofidry

Problem

Give the following configuration file:

<invalid>
</invalid>

PHPUnit 12.5.21 (and older versions, I tested down to 11.5.55 at least), only display the same output as if you did

phpunit --help
Executing PHPUnit with an invalid configuration file
$ echo '<invalid></invalid>' > phpunit.xml
$ phpunit

PHPUnit 12.5.21 by Sebastian Bergmann and contributors.

Usage:
  phpunit [options] <directory|file> ...

Configuration:

  --bootstrap <file>                   A PHP script that is included before the tests run
  -c|--configuration <file>            Read configuration from XML file
  --no-configuration                   Ignore default configuration file (phpunit.xml)
  --extension <class>                  Register test runner extension with bootstrap <class>
  --no-extensions                      Do not register test runner extensions
  --include-path <path(s)>             Prepend PHP's include_path with given path(s)
  -d <key[=value]>                     Sets a php.ini value
  --cache-directory <dir>              Specify cache directory
  --generate-configuration             Generate configuration file with suggested settings
  --migrate-configuration              Migrate configuration file to current format
  --generate-baseline <file>           Generate baseline for issues
  --use-baseline <file>                Use baseline to ignore issues
  --ignore-baseline                    Do not use baseline to ignore issues

Selection:

  --all                                Ignore test selection from XML configuration file
  --list-suites                        List available test suites
  --testsuite <name>                   Only run tests from the specified test suite(s)
  --exclude-testsuite <name>           Exclude tests from the specified test suite(s)
  --list-groups                        List available test groups
  --group <name>                       Only run tests from the specified group(s)
  --exclude-group <name>               Exclude tests from the specified group(s)
  --covers <name>                      Only run tests that intend to cover <name>
  --uses <name>                        Only run tests that intend to use <name>
  --requires-php-extension <name>      Only run tests that require PHP extension <name>
  --list-test-files                    List available test files
  --list-tests                         List available tests
  --list-tests-xml <file>              List available tests in XML format
  --filter <pattern>                   Filter which tests to run
  --exclude-filter <pattern>           Exclude tests for the specified filter pattern
  --test-suffix <suffixes>             Only search for test in files with specified suffix(es). Default: Test.php,.phpt

Execution:

  --process-isolation                  Run each test in a separate PHP process
  --globals-backup                     Backup and restore $GLOBALS for each test
  --static-backup                      Backup and restore static properties for each test

  --strict-coverage                    Be strict about code coverage metadata
  --strict-global-state                Be strict about changes to global state
  --disallow-test-output               Be strict about output during tests
  --enforce-time-limit                 Enforce time limit based on test size
  --default-time-limit <sec>           Timeout in seconds for tests that have no declared size
  --do-not-report-useless-tests        Do not report tests that do not test anything

  --stop-on-defect                     Stop after first error, failure, warning, or risky test
  --stop-on-error                      Stop after first error
  --stop-on-failure                    Stop after first failure
  --stop-on-warning                    Stop after first warning
  --stop-on-risky                      Stop after first risky test
  --stop-on-deprecation                Stop after first test that triggered a deprecation
  --stop-on-notice                     Stop after first test that triggered a notice
  --stop-on-skipped                    Stop after first skipped test
  --stop-on-incomplete                 Stop after first incomplete test

  --fail-on-empty-test-suite           Signal failure using shell exit code when no tests were run
  --fail-on-warning                    Signal failure using shell exit code when a warning was triggered
  --fail-on-risky                      Signal failure using shell exit code when a test was considered risky
  --fail-on-deprecation                Signal failure using shell exit code when a deprecation was triggered
  --fail-on-phpunit-deprecation        Signal failure using shell exit code when a PHPUnit deprecation was triggered
  --fail-on-phpunit-notice             Signal failure using shell exit code when a PHPUnit notice was triggered
  --fail-on-phpunit-warning            Signal failure using shell exit code when a PHPUnit warning was triggered
  --fail-on-notice                     Signal failure using shell exit code when a notice was triggered
  --fail-on-skipped                    Signal failure using shell exit code when a test was skipped
  --fail-on-incomplete                 Signal failure using shell exit code when a test was marked incomplete
  --fail-on-all-issues                 Signal failure using shell exit code when an issue is triggered

  --do-not-fail-on-empty-test-suite    Do not signal failure using shell exit code when no tests were run
  --do-not-fail-on-warning             Do not signal failure using shell exit code when a warning was triggered
  --do-not-fail-on-risky               Do not signal failure using shell exit code when a test was considered risky
  --do-not-fail-on-deprecation         Do not signal failure using shell exit code when a deprecation was triggered
  --do-not-fail-on-phpunit-deprecation Do not signal failure using shell exit code when a PHPUnit deprecation was triggered
  --do-not-fail-on-phpunit-notice      Do not signal failure using shell exit code when a PHPUnit notice was triggered
  --do-not-fail-on-phpunit-warning     Do not signal failure using shell exit code when a PHPUnit warning was triggered
  --do-not-fail-on-notice              Do not signal failure using shell exit code when a notice was triggered
  --do-not-fail-on-skipped             Do not signal failure using shell exit code when a test was skipped
  --do-not-fail-on-incomplete          Do not signal failure using shell exit code when a test was marked incomplete

  --cache-result                       Write test results to cache file
  --do-not-cache-result                Do not write test results to cache file

  --order-by <order>                   Run tests in order: default|defects|depends|duration|no-depends|random|reverse|size
  --resolve-dependencies               Alias for "--order-by depends"
  --ignore-dependencies                Alias for "--order-by no-depends"
  --random-order                       Alias for "--order-by random"
  --random-order-seed <N>              Use the specified random seed when running tests in random order
  --reverse-order                      Alias for "--order-by reverse"

Reporting:

  --colors=<flag>                      Use colors in output ("never", "auto" or "always")
  --columns <n>                        Number of columns to use for progress output
  --columns max                        Use maximum number of columns for progress output
  --stderr                             Write to STDERR instead of STDOUT

  --no-progress                        Disable output of test execution progress
  --no-results                         Disable output of test results
  --no-output                          Disable all output

  --display-incomplete                 Display details for incomplete tests
  --display-skipped                    Display details for skipped tests
  --display-deprecations               Display details for deprecations triggered by tests
  --display-phpunit-deprecations       Display details for PHPUnit deprecations
  --display-phpunit-notices            Display details for PHPUnit notices
  --display-errors                     Display details for errors triggered by tests
  --display-notices                    Display details for notices triggered by tests
  --display-warnings                   Display details for warnings triggered by tests
  --display-all-issues                 Display details for all issues that are triggered
  --reverse-list                       Print defects in reverse order

  --teamcity                           Replace default progress and result output with TeamCity format
  --testdox                            Replace default result output with TestDox format
  --testdox-summary                    Repeat TestDox output for tests with errors, failures, or issues

  --debug                              Replace default progress and result output with debugging information
  --with-telemetry                     Include telemetry information in debugging information output

Logging:

  --log-junit <file>                   Write test results in JUnit XML format to file
  --log-otr <file>                     Write test results in Open Test Reporting XML format to file
  --include-git-information            Include Git information in Open Test Reporting XML logfile
  --log-teamcity <file>                Write test results in TeamCity format to file
  --testdox-html <file>                Write test results in TestDox format (HTML) to file
  --testdox-text <file>                Write test results in TestDox format (plain text) to file
  --log-events-text <file>             Stream events as plain text to file
  --log-events-verbose-text <file>     Stream events as plain text with extended information to file
  --no-logging                         Ignore logging configured in the XML configuration file

Code Coverage:

  --coverage-clover <file>             Write code coverage report in Clover XML format to file
  --coverage-openclover <file>         Write code coverage report in OpenClover XML format to file
  --coverage-cobertura <file>          Write code coverage report in Cobertura XML format to file
  --coverage-crap4j <file>             Write code coverage report in Crap4J XML format to file
  --coverage-html <dir>                Write code coverage report in HTML format to directory
  --coverage-php <file>                Write serialized code coverage data to file
  --coverage-text=<file>               Write code coverage report in text format to file [default: standard output]
  --only-summary-for-coverage-text     Option for code coverage report in text format: only show summary
  --show-uncovered-for-coverage-text   Option for code coverage report in text format: show uncovered files
  --coverage-xml <dir>                 Write code coverage report in XML format to directory
  --exclude-source-from-xml-coverage   Exclude <source> element from code coverage report in XML format
  --warm-coverage-cache                Warm static analysis cache
  --coverage-filter <dir>              Include <dir> in code coverage reporting
  --path-coverage                      Report path coverage in addition to line coverage
  --disable-coverage-ignore            Disable metadata for ignoring code coverage
  --no-coverage                        Ignore code coverage reporting configured in the XML configuration file

Miscellaneous:

  -h|--help                            Prints this usage information
  --version                            Prints the version and exits
  --atleast-version <min>              Checks that version is greater than <min> and exits
  --check-version                      Checks whether PHPUnit is the latest version and exits
  --check-php-configuration            Checks whether PHP configuration follows best practices

I am not up to date with the expected user experience for the different degrees of invalid PHPUnit configuration files, neither what really happens if there is no XSD specified or a custom one provided.

Context

The issue was brought up by infection/infection#2303.

In infection, for the initial tests (and only the initial ones), we validate the PHPUnit configuration file.

How infection validates the configuration file
if ($xPath->queryCount('/phpunit') === 0) {
    throw InvalidPhpUnitConfiguration::byRootNode($configPath);
}

if ($xPath->queryCount('namespace::xsi') === 0) {
    return true;
}

$schema = $xPath->queryAttribute('/phpunit/@xsi:noNamespaceSchemaLocation')?->nodeValue;

$original = libxml_use_internal_errors(true);

if ($schema !== null && !$xPath->document->schemaValidate($this->buildSchemaPath($schema))) {
    throw InvalidPhpUnitConfiguration::byXsdSchema(
        $configPath,
        $this->getXmlErrorsString(),
    );
}

libxml_use_internal_errors($original);

This helps providing a better feedback to the user when the configuration file is invalid (this happens before any modification is done by infection).

As such, this validation step feels a bit redundant. On one hand, there is still value in it because otherwise the modifications that we will do (done by loading an XPath) may be incorrect and fail weirdly, on the other hand, it feels wrong to have to care about this. We are dealing with the PHPUnit TestFramework Adapter here, so that it needs to deal with some specificities and implementation details of PHPUnit is expected to a degree, but I am questioning if this specific step is not going overboard, and evidently fail to capture some of the logic.

In #2303 we decided that PHPUnit already validates the file, there is no need for us to repeat that work and I attempted to remove the XSD validation in infection/infection#3042.

My findings are that:

  • The output is a lot less clear, because we treat this as any PHPUnit failure, so we output the command we executed and the whole command output.
  • In some cases, like the first example, the output is quite unclear. The user can infer there is something wrong there, but it lacks clarity.

Considered solutions

  • As a PHPUnit user I feel that an incorrect configuration (to be differentiated with an absence of a configuration file) should cause a clear failure. Displaying the help command feels... unhelpful? But I appreciate I may be lacking context here.
  • Ideally, PHPUnit would fail with a specific error code in this specific scenario (an invalid configuration file). This way, orchestrating tools could adapt the feedback provided by the user based on a clear piece of information, rather than either simply outputting the whole output (which may lack clarity) or trying to infer what happened based on the output (brittle).

Metadata

Metadata

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions