Edit report at https://bugs.php.net/bug.php?id=76833&edit=1

 ID:                 76833
 Updated by:         a...@php.net
 Reported by:        c...@php.net
 Summary:            stream_socket_enable_crypto-win32.phpt fails
 Status:             Open
 Type:               Bug
 Package:            Testing related
 Operating System:   Windows
 PHP Version:        7.2Git-2018-09-01 (Git)
 Block user comment: N
 Private report:     N

 New Comment:

Yeah, this test has been a headache for some time already. And it's a bit 
controversial, as creating a socket server and then enabling crypto as a client 
seems quite odd. Such tests make sense for testing abnormal situation, but in 
this case it requires too much maintanance. I'll check to backport these 
changes from 7.3, otherwise this test should be removed.

Thanks.


Previous Comments:
------------------------------------------------------------------------
[2018-09-01 17:29:57] ka...@php.net

Yeah I was a little curious on that when I was reading over the code, it does 
indeed seem like it was just for coverage. At least with our more recent point 
of view in regards to these, we should probably remove the test by itself.

As for testing actual relevant stuff, I guess what we could do for streams 
stuff, is to maybe make a forked version of already existing tests to enable 
crypto stuff where it makes sense unless someone steps up to specifically write 
such.

------------------------------------------------------------------------
[2018-09-01 16:12:17] c...@php.net

Yes, master and 7.3 are apparently okay.  Regarding the localized
message: it actually doesn't make sense to raise E_WARNING for
something that succeeded.  Anyhow, now that I had a closer look at
the test[1], I don't think it makes much sense at all.  Looks like
the test tries to increase code coverage, without testing the
actually relevant stuff (for instance, that after enabling crypto
the stream still “works”).

[1] 
<https://github.com/php/php-src/blob/php-7.3.0beta3/ext/standard/tests/streams/stream_socket_enable_crypto.phpt>

------------------------------------------------------------------------
[2018-09-01 15:01:48] ka...@php.net

It seems like Caruso merged this Windows specific test with the Unix 
counterpart for master and you are right with the language specific error 
message, perhaps we should use %s in place there as the error error messages 
seems to come from the function itself.

As for the error itself, that I did not have enough time to dig into here, but 
does the master one pass?

------------------------------------------------------------------------
[2018-09-01 14:24:19] c...@php.net

Description:
------------
ext\standard\tests\streams\stream_socket_enable_crypto-win32.phpt
fails with openssl-1.1.0i-vc15-x64 on PHP-7.2, as can be seen on
Appveyor[1] (I can also reproduce this locally).  With older
openssl the test generally succeeds, but on a German system it
fails with the following diff:

012+ Warning: stream_socket_enable_crypto(): SSL: Der Vorgang wurde erfolgreich 
beendet.
012- Warning: stream_socket_enable_crypto(): SSL: The operation completed 
successfully.

[1] 
<https://ci.appveyor.com/project/php/php-src/build/PHP-7.2.build.8414/job/ay1uiffrme2rf5n8#L5512>



------------------------------------------------------------------------



--
Edit this bug report at https://bugs.php.net/bug.php?id=76833&edit=1

Reply via email to