Test online tests in CI #8390

mvorisek · 2022-04-17T10:21:10Z

CI setup already heavily relies on internet connection, so run online tests in CI too.

mvorisek · 2022-04-17T10:48:18Z

there is one failing test on MacOS, is it fixable or should the test be skipped on MacOS?

mvorisek · 2022-04-20T08:50:27Z

this online test is failing often:

 ========DIFF========
001+ Warning: XMLReader::setRelaxNGSchema(http://docs.oasis-open.org/docbook/rng/5.0/docbook.rng): Failed to open stream: HTTP request failed! HTTP/1.1 500 Internal Server Error

002+  in D:\a\php-src\php-src\ext\xmlreader\tests\bug70309.php on line 30

003+ 

004+ Warning: XMLReader::setRelaxNGSchema(): I/O warning : failed to load external entity "http://docs.oasis-open.org/docbook/rng/5.0/docbook.rng" in D:\a\php-src\php-src\ext\xmlreader\tests\bug70309.php on line 30

005+ 

006+ Warning: XMLReader::setRelaxNGSchema(): xmlRelaxNGParse: could not load http://docs.oasis-open.org/docbook/rng/5.0/docbook.rng in D:\a\php-src\php-src\ext\xmlreader\tests\bug70309.php on line 30

007+ 

008+ Warning: XMLReader::setRelaxNGSchema(): Schema contains errors in D:\a\php-src\php-src\ext\xmlreader\tests\bug70309.php on line 30

     ===DONE===
========DONE========
FAIL XMLReader: Bug #70309 XmlReader read generates extra output [D:\a\php-src\php-src\ext\xmlreader\tests\bug70309.phpt]

can the test file be downloaded and server using a local test server?

cmb69 · 2022-04-20T10:15:46Z

can the test file be downloaded and server using a local test server?

I'm not even sure that we need to keep this test. It is a regression test for an upstream bug which has been fixed some years ago.

If we want to keep the test, we likely could use a local copy of docbook.rng (would need to double-check its license), but we also could rewrite the test to use some simpler *.rng (it's just about using a non-supported tag name).

mvorisek · 2022-04-20T10:54:52Z

I will remove the test, fixed years ago, for Windows only and also never an issue with php-src code itself.

cmb69 · 2022-04-20T11:09:00Z

[…] for Windows only […]

The bug affected all systems, and has been fixed upstream in the meantime.

mvorisek · 2022-04-20T11:17:32Z

The bug affected all systems, and has been fixed upstream in the meantime.

yes, with other part of my head on the Windows CI PR... :D

About the failing test on MacOS:


========DIFF========
002+ 
003+ Warning: socket_send(): Unable to write to socket [22]: Invalid argument in /Users/runner/work/php-src/php-src/ext/sockets/tests/socket_send.php on line 14
     okey
     okey
004- okey
========DONE========
FAIL int socket_send ( resource $socket , string $buf , int $len , int $flags ); [ext/sockets/tests/socket_send.phpt]

can the support be fixed easily or should the test with MSG_EOR be skipped on MacOS?

cmb69 · 2022-04-20T11:27:16Z

About the failing test on MacOS:


========DIFF========
002+ 
003+ Warning: socket_send(): Unable to write to socket [22]: Invalid argument in /Users/runner/work/php-src/php-src/ext/sockets/tests/socket_send.php on line 14
     okey
     okey
004- okey
========DONE========
FAIL int socket_send ( resource $socket , string $buf , int $len , int $flags ); [ext/sockets/tests/socket_send.phpt]

can the support be fixed easily or should the test with MSG_EOR be skipped on MacOS?

I wouldn't know. Maybe @devnexen does?

mvorisek · 2022-04-22T20:31:46Z

ext/sockets/sockets.c

@@ -482,7 +482,7 @@ static PHP_MINIT_FUNCTION(sockets)
 	REGISTER_LONG_CONSTANT("MSG_TRUNC",		MSG_TRUNC,		CONST_CS | CONST_PERSISTENT);
 	REGISTER_LONG_CONSTANT("MSG_PEEK",		MSG_PEEK,		CONST_CS | CONST_PERSISTENT);
 	REGISTER_LONG_CONSTANT("MSG_DONTROUTE", MSG_DONTROUTE,	CONST_CS | CONST_PERSISTENT);
-#ifdef MSG_EOR
+#if defined(MSG_EOR) && !defined(__APPLE__)


google for MSG_EOR macos

it seems it is either completely unsupported or it needs SEQPACKET, but it is not used by php

if someone knows better, please advise

@devnexen do you know?

We cannot change this (definitely not for any of the stable versions, but I don't see a compelling reason to change it for master either). The userland constant should be defined on macOS as well (even if it wouldn't make sense there).

What we likely should change is socket_send.phpt; it seems to me that the MSG_EOR case only works by accident (where it works), since socket_send() is a wrapper for send(2), and POSIX specifies:

MSG_EOR
Terminates a record (if supported by the protocol).

The Linux send manpage clarifies:

MSG_EOR (since Linux 2.2)
Terminates a record (when this notion is supported, as for
sockets of type SOCK_SEQPACKET).

However, the socket we're using here is of type SOCK_STREAM.

It would be good if someone experienced with sockets could review that test; and we should reconsider connecting to a domain which we don't control (yahoo.com).

@devnexen ext/sockets/tests/socket_send.phpt test is failing on MacOS (not in CI, as online tests are skipped), do you think the code can be fixed or the MSG_EOR test be skipped like in https://github.com/php/php-src/pull/8390/files#diff-a516d52fc571b5da772a0ff19826e9bd371e3562eab95ca4a2b71108dda1da8bR26?

cmb69

See #8390 (comment).

github-actions · 2022-07-26T00:02:52Z

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.

mvorisek · 2022-07-26T00:31:35Z

not stale

This reverts commit 52871d8.

bukka · 2022-08-25T18:21:44Z

I am not in favor of this as it might depend on external resources that might eventually stop working. In general we should more look into replacing the online tests as often those tests could be done differently and not require it.

mvorisek · 2022-08-31T11:22:24Z

I am not in favor of this as it might depend on external resources that might eventually stop working. In general we should more look into replacing the online tests as often those tests could be done differently and not require it.

This is quite recursive statement. Of course, no external/online tests would be the best. But in a situation they exist, I found them better to be executed as long as there are no frequent random CI failures because of them. Can we aggree on this @bukka or do you have some reason why we should close this PR instead?

github-actions · 2022-10-31T00:03:26Z

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.

mvorisek changed the base branch from master to PHP-8.0 April 17, 2022 10:21

mvorisek force-pushed the test_online_tests_in_ci branch from 81b5407 to 0393ad0 Compare April 17, 2022 10:21

mvorisek closed this Apr 17, 2022

mvorisek deleted the test_online_tests_in_ci branch April 17, 2022 12:10

mvorisek restored the test_online_tests_in_ci branch April 17, 2022 12:11

mvorisek reopened this Apr 17, 2022

mvorisek force-pushed the test_online_tests_in_ci branch from 0393ad0 to 2d39ea4 Compare April 20, 2022 11:05

mvorisek mentioned this pull request Apr 22, 2022

Test Windows x86 and x64 with Github Actions #8392

Closed

mvorisek marked this pull request as draft April 22, 2022 18:59

mvorisek commented Apr 22, 2022

View reviewed changes

mvorisek marked this pull request as ready for review April 26, 2022 12:53

cmb69 requested changes May 18, 2022

View reviewed changes

ramsey added Category: Tests Waiting on Author labels May 22, 2022

github-actions bot added the Stale label Jul 26, 2022

github-actions bot removed the Stale label Jul 27, 2022

mvorisek force-pushed the test_online_tests_in_ci branch from a02327a to 4a643cf Compare August 25, 2022 12:41

mvorisek added 4 commits August 25, 2022 15:45

Test online tests in CI

c3f45be

MSG_EOR is not supported on MacOS even if defined

6b5b864

Revert "MSG_EOR is not supported on MacOS even if defined"

57a5492

This reverts commit 52871d8.

fix test for MacOS instead

d14c312

mvorisek force-pushed the test_online_tests_in_ci branch from 4a643cf to d14c312 Compare August 25, 2022 13:45

github-actions bot added Stale and removed Stale labels Oct 31, 2022

mvorisek changed the base branch from PHP-8.0 to PHP-8.1 November 12, 2022 14:07

mvorisek closed this Nov 12, 2022

mvorisek deleted the test_online_tests_in_ci branch November 12, 2022 16:33

bukka mentioned this pull request Jan 1, 2025

Rewrite http gh16810 test to not be online #17314

Closed

Test online tests in CI #8390

Test online tests in CI #8390

Uh oh!

Conversation

mvorisek commented Apr 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mvorisek commented Apr 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mvorisek commented Apr 20, 2022

Uh oh!

cmb69 commented Apr 20, 2022

Uh oh!

mvorisek commented Apr 20, 2022

Uh oh!

cmb69 commented Apr 20, 2022

Uh oh!

mvorisek commented Apr 20, 2022

Uh oh!

cmb69 commented Apr 20, 2022

Uh oh!

mvorisek Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvorisek Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

cmb69 May 12, 2022

Choose a reason for hiding this comment

Uh oh!

mvorisek Aug 25, 2022

Choose a reason for hiding this comment

Uh oh!

cmb69 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 26, 2022

Uh oh!

mvorisek commented Jul 26, 2022

Uh oh!

bukka commented Aug 25, 2022

Uh oh!

mvorisek commented Aug 31, 2022

Uh oh!

github-actions bot commented Oct 31, 2022

Uh oh!

Uh oh!

mvorisek commented Apr 17, 2022 •

edited

Loading

mvorisek commented Apr 17, 2022 •

edited

Loading

mvorisek Apr 22, 2022 •

edited

Loading