Bug 268447 - PHP errors ignored in PHPUnit test results
PHP errors ignored in PHPUnit test results
Status: VERIFIED FIXED
Product: php
Classification: Unclassified
Component: PHPUnit
8.2
All All
: P2 (vote)
: 8.2
Assigned To: Tomas Mysik
issues@php
82patch1-fixed
: REGRESSION
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-10 11:50 UTC by mwvdlee
Modified: 2016-12-03 14:06 UTC (History)
0 users

See Also:
Issue Type: DEFECT
:


Attachments
badTest.php (139 bytes, application/octet-stream)
2016-10-10 14:19 UTC, mwvdlee
Details

Note You need to log in before you can comment on or make changes to this bug.
Description mwvdlee 2016-10-10 11:50:34 UTC
Reproduce; any PHPUnit testcase with a PHP Fatal Error.
Actual: Test passed
Expected: Test failed

Since Netbeans 8.2 (worked fine in 8.1), PHP errors are not reported as failed tests in the "Test Results" window. The error message with callstack is dumped correctly in the "Output" window.
Comment 1 Tomas Mysik 2016-10-10 12:02:26 UTC
Could you please attach a sample file which reproduces this problem?

Thanks.
Comment 2 mwvdlee 2016-10-10 14:19:47 UTC
Created attachment 162432 [details]
badTest.php
Comment 3 mwvdlee 2016-10-10 14:21:31 UTC
I've attached a file that causes this problem.

It reaches the nesting level, triggers a fatal error and does not reach the assertion.

Netbeans reports the test as passed succesfully.

<code>
class badTest extends PHPUnit_Framework_TestCase {
	public function testBad() {
		$this->testBad();
		$this->assertTrue(false);
	}
}
</code>
Comment 4 mwvdlee 2016-10-11 09:48:16 UTC
Seems that MySQL errors are also not detected as errors by Netbeans 8.2.

An `SQL Error #1064 'You have an error in your SQL syntax; check the manual ...` error is also passed as succesfully.

This was followed by a PHP error `Fatal error: Call to a member function fetch_assoc() on a non-object in ...` that also passed the unittest incorrectly.
Comment 5 Tomas Mysik 2016-10-13 06:16:03 UTC
Hmm, unfortunately this cannot be fixed on NetBeans' side :/ The problem is that PHPUnit does not report this error. I have reported it against PHPUnit [1], will watch it.

Thanks for reporting.
[1] https://github.com/sebastianbergmann/phpunit/issues/2325
Comment 6 Tomas Mysik 2016-10-13 06:23:43 UTC
Reopening, will try PHPUnit 5.x next week.

Thanks.
Comment 7 Tomas Mysik 2016-10-13 06:24:00 UTC
(My current version is PHPUnit 4.8.23).
Comment 8 mwvdlee 2016-10-13 14:39:34 UTC
Thanks for reporting it to PHPUnit!

Could it be that the mechanism for checking results has been changed in Netbeans 8.2? I noticed the test result window is now showing results in real time and that performance seems to be much lower when running lots of small tests. That would explain why this wasn't an issue in 8.1
Comment 9 Tomas Mysik 2016-10-14 06:27:58 UTC
(In reply to mwvdlee from comment #8)
> Thanks for reporting it to PHPUnit!

You are welcome, of course.

> Could it be that the mechanism for checking results has been changed in
> Netbeans 8.2?

Yes, NB 8.2 switched to continuous tests results. NB 8.1 and older used XML report so the results could be presented after the test run finished, not earlier.

> I noticed the test result window is now showing results in
> real time and that performance seems to be much lower when running lots of
> small tests. That would explain why this wasn't an issue in 8.1

Is the performance really worse? Would it be possible to provide some times or a short video? Or a sample project we could use for comparison?

Thanks!
Comment 10 mwvdlee 2016-10-14 21:18:41 UTC
> Is the performance really worse? Would it be possible to provide some times
> or a short video? Or a sample project we could use for comparison?

So far I've seen only one of my test suites that went from a couple of seconds to roughly half a minute. I may be able to make a video (assuming you'd like both a NB8.1 and 8.2 comparison), but a sample project is going to be difficult, as the suite in question is for my job. I'll check my open source code to see if one of them has the same problem.
Comment 11 mwvdlee 2016-10-15 07:21:02 UTC
Based on the response on https://github.com/sebastianbergmann/phpunit/issues/2325, I gather the best thing to do would be to either offer a switch for continuous test results or only use continuous if PHP7+ is detected.
Comment 12 Tomas Mysik 2016-10-17 05:38:08 UTC
(In reply to mwvdlee from comment #11)
> Based on the response on
> https://github.com/sebastianbergmann/phpunit/issues/2325, I gather the best
> thing to do would be to either offer a switch for continuous test results or
> only use continuous if PHP7+ is detected.

Sebastian wrote "Every log is unreliable when a fatal error occurs in PHP 5.", therefore I do not think this switch would help (maybe in this particular case but not in general).

Thanks.
Comment 13 mwvdlee 2016-10-17 14:03:29 UTC
> Sebastian wrote "Every log is unreliable when a fatal error occurs in PHP
> 5.", therefore I do not think this switch would help (maybe in this
> particular case but not in general).

I've compared 8.1 with 8.2 on the same testfile I posted earlier, both using PHPUnit 4.8.24.

Netbeans 8.1 test results window shows a gray bar (Test passed 0,00%) reports "No tests executed.", with "Perhaps error occurred, verify in Output window." in the log pane in red text.

Netbeans 8.2. shows a green bar (Tests passed; 100,00%), reports "The test passed." and has "Full output can be found in Output window." as normal colored text in the log pane.

I've also compared the slow testsuite: Seems 8.2 takes a long time initially, but runs a bit faster after the first time.
199 tests in 8.2 runs in ~138s initially, consecutive runs ~75s on average.
8.1 runs about ~80s on average every time. Turns out this isn't a big issue.

Netbeans 8.2. also does not seem to report the full results of assertions in the "Test Results" window's log pane, nor does it recognize as many code "hyperlinks". 8.2. also has some issues with correctly identifying the test suites.

Seems like the unittesting featuring could use some unittesting ;) I really like to multi-carets feature, but I use unittesting a lot more. So I'm moving back to Netbeans 8.1 for now.
Comment 14 Tomas Mysik 2016-10-18 11:57:49 UTC
(In reply to Tomas Mysik from comment #6)
> Reopening, will try PHPUnit 5.x next week.

So, PHPUnit 5.6.1 together with PHP 7.0.8 does not help, the log is basically the same (it means incomplete). I updated the PHPUnit issue on GitHub.

Thanks.
Comment 15 Tomas Mysik 2016-10-18 12:00:20 UTC
I will review the JSON parser in NetBeans and will try to improve it - if the test result is missing in the log, marking the test as failed should be better but not sure if it is so easy. Will investigate it more later.

Thanks.
Comment 16 elGrizlo 2016-11-04 12:30:38 UTC
In previous versions (<8.2) test log was written as xml (--log-junit), which was zero size if fatal error occured. Now it writes json (--log-json), which breaks at fatal line.

Normal test's events sequence:


{
    "event": "suiteStart",
    "suite": "BrioCore\\Sphinx\\FactorServiceTest",
    "tests": 1
}{
    "event": "testStart",
    "suite": "BrioCore\\Sphinx\\FactorServiceTest",
    "test": "BrioCore\\Sphinx\\FactorServiceTest::testParseFields"
}{
    "event": "test",
    "suite": "BrioCore\\Sphinx\\FactorServiceTest",
    "test": "BrioCore\\Sphinx\\FactorServiceTest::testParseFields",
    "status": "pass",
    "time": 0.0035998821258545,
    "trace": [],
    "message": "",
    "output": ""
}




if something causes fatal in testParseFields , last event will be
{
    "event": "suiteStart",
    "suite": "BrioCore\\Sphinx\\FactorServiceTest",
    "tests": 1
}{
    "event": "testStart",
    "suite": "BrioCore\\Sphinx\\FactorServiceTest",
    "test": "BrioCore\\Sphinx\\FactorServiceTest::testParseFields"
}

EOF


So, It must be checked if every event: "testStart" has following event:"test"


Hope, it will be fixed in 8.2.1 :-)
Comment 17 Tomas Mysik 2016-11-11 13:14:14 UTC
Should be fixed now, I hope, the best way we can do, I believe. If any test is started but did not finish, NetBeans reports PHP fatal error (and the test itself has error status).

Please, verify it, it is important in order to be part of the patch for NB 8.2.

Thanks!

http://hg.netbeans.org/web-main/rev/e8ab2e182a93
Comment 18 mwvdlee 2016-11-11 13:51:30 UTC
(In reply to Tomas Mysik from comment #17)
> Please, verify it, it is important in order to be part of the patch for NB
> 8.2.

I'm not a Netbeans developer. How would I go about testing this?
Comment 19 Tomas Mysik 2016-11-11 15:00:29 UTC
(In reply to mwvdlee from comment #18)
> I'm not a Netbeans developer. How would I go about testing this?

Wait for an automatic message that will appear here saying in which daily build this fix will be present. It should not take so long (likely today or tomorrow).

Thanks!
Comment 20 Quality Engineering 2016-11-13 02:53:24 UTC
Integrated into 'main-silver', will be available in build *201611130001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/e8ab2e182a93
User: Tomas Mysik <tmysik@netbeans.org>
Log: #268447 - PHP errors ignored in PHPUnit test results
Comment 21 mwvdlee 2016-11-13 09:02:40 UTC
(In reply to Quality Engineering from comment #20)
> Integrated into 'main-silver', will be available in build *201611130001* on
> http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
> 
> Changeset: http://hg.netbeans.org/main-silver/rev/e8ab2e182a93
> User: Tomas Mysik <tmysik@netbeans.org>
> Log: #268447 - PHP errors ignored in PHPUnit test results

Installed and tested this build.
It works.
I now get a nice big error message telling me a test had a fatal error and reporting it as failed.
Comment 22 Tomas Mysik 2016-11-14 06:32:01 UTC
Thanks a lot for verification!
Comment 23 Tomas Mysik 2016-12-03 14:06:57 UTC
Transplanted to the releases repo branch release82:

http://hg.netbeans.org/releases/rev/579c52645067

Thanks.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo