Problem/Motivation
Currently, phpunit tests are run via the SimpleTest test runner.
I think we could speed up test runs - at least in cases where unit tests fail, by running these separately. If we run PHPUnit tests first, and bail out with a failure reported to the issue/CNW immediately, it would save doing SimpleTest at all for those patches.
Proposed resolution
Determine how to get the phpunit test runner to write JUnit XML files with PHP fatal.
Determine the suitability of the existing PHPUnit job: http://cgit.drupalcode.org/drupalci_testbot/tree/src/DrupalCI/Plugin/Job...
Modify d.o integration so that it can run the phpunit job independently.
Comments
Comment #1
Wim LeersPlus, in cases like https://qa.drupal.org/pifr/test/737993, where there's a PHP fatal, but it's impossible to deduce from the review log in WHICH test the PHP fatal occurs, even adding debug output (like https://qa.drupal.org/pifr/test/738043) yields completely useless results. It was Berdir who suggested to me that it might be a PHPUnit test failure, so I ran the entire PHPUnit test suite, and voila, there it was, including a stacktrace, so I at least was able to figure out how the test failure occurred.
So not only can we speed things up, we'll also make failed tests a lot more debuggable by not letting SimpleTest run PHPUnit tests — which appears to strip all useful information.
Comment #2
damiankloip CreditAttribution: damiankloip commentedYes indeed, if we get a fatal in a phpunit test from the bot we get pretty much nothing. Only if you already know about this do you know that running phpunit locally will show you the problem :/
Comment #3
isntall CreditAttribution: isntall at Drupal Association commentedComment #4
Mile23drupalci_testbot currently defines a phpunit job type but nothing runs it: http://cgit.drupalcode.org/drupalci_testbot/tree/src/DrupalCI/Plugin/Job...
The test definition file should specify using this job.
The drupalci_jenkins project is not where this should be filed... Moving to the test runner for now.
Comment #5
dawehnerWell the problem is that when a phpunit process fails, it doesn't give you any helpful XML output, unlike with our run-tests.sh wrapper, this is not the case.
Comment #6
jthorson CreditAttribution: jthorson commented#5 is correct ... the initial implementation of the test runner on D.O. leveraged the existing xml output from simpletest, as the intended design was not built into the test runner yet. The initial intended design would see a 'generic' result object which includes things common to most jobs, such as pass/fail/error state, console output, a human readable 'job result' summary string, the job build parameters and definition used to run the test, and an array of 0..n files which would contain the results that are specific to that particular job type. Each job type would then be expected to generate it's output in alignment with this structure.
But because this wasn't built yet, and due to the rush to get DrupalCI in and shut down PIFR, today's d.o DrupalCI integration was based directly to the existing testing xml output. To truly be able to support multiple independent job types, as per the initial visioin, we need to either i) complete the abstraction of results and provide them out of the test runner directly (i.e. removing our dependency on jenkins-based xml as the only format accepted by d.o), or ii) hack all other job types into reporting via the xml format currently being used.
Comment #7
Mile23OK, so closing this.
We should perhaps think about having a way to run low-dependency tests (such as unit tests) first, and allowing an option for quit-on-first-fail, regardless of how the tests are run.
Comment #8
jthorson CreditAttribution: jthorson commentedDon't close this ... The vision was always for the quick-and-dirty code review and phpunit tests to be run first, with the more expensive Simpletest tests done either in parallel, or only triggered after the rest pass. I'd still like to see it!
Comment #9
Mile23That's not in scope for this issue, though. This issue is about running the phpunit executable, which apparently we can't do without an upstream change.
Also, D8 has integration tests in phpunit, which are about as slow as their simpletest counterparts, so it's no longer true that phpunit tests are super-quick.
Here's a meta I just made for all the coding standards and linting stuff: #2654650: [meta] Jobs for linting and coding standards
Comment #10
jthorson CreditAttribution: jthorson commentedWithin the context of the DrupalCI queue, as this issue is currently filed, I consider this "get a PHPUnit job type running that is separate and independent from the Simpletest Job type, and fix the d.o integration so that we can run the PHPUnit tests independently".
If it needs an upstream change, then I'd support a status of 'postponed', with a pointer to the upstream change which is required ... but I wouldn't close off a very desirable future state feature with "Works as designed".
Comment #11
Mile23Comment #12
Mile23Comment #13
Mile23Still blocked by this stuff: #2654650: [meta] Jobs for linting and coding standards
Basically we need to review docker-based patches and also address the code base.
Comment #14
MixologicComment #15
Mile23The specifics for adding a PHPUnit test definition: #2189317: Get a PHPUnit job type running that is separate and independent from the Simpletest Job type, fix the d.o integration so that we can run the PHPUnit tests independently
Comment #16
Mile23Ewps. Wrong issue. This one: #2837112: Add a test definition for a generic PHPUnit test run
Comment #17
MixologicThere is a good start of this over at 2466173-dogfood branch