reorganize http tests; avoid double execution of legacy loader test; test comments and cleanups

Author: gggeek

.github/workflows/ci.yaml

@@ -116,9 +116,6 @@ jobs:
                         fi
                     fi
 
-            -   name: test legacy php classes loader
-                run: 'php ./tests/legacy_loader_test.php'
-
             # @todo would it be useful to run a 2nd test with composer --prefer-lowest? After all the only dependencies we have are testing tools
 
             -   name: test python demo files

tests/01CharsetTest.php

@@ -101,7 +101,6 @@ public function testLatin15()
     {
         if (!function_exists('mb_convert_encoding')) {
             $this->markTestSkipped('Miss mbstring extension to test exotic charsets');
-            return;
         }
 
         // euro symbol in ISO-8859-15

tests/02ValueTest.php

@@ -219,7 +219,6 @@ public function testLatin15InternalEncoding()
     {
         if (!function_exists('mb_convert_encoding')) {
             $this->markTestSkipped('Miss mbstring extension to test exotic charsets');
-            return;
         }
 
         $string = chr(164);

tests/04ParsingTest.php

@@ -103,7 +103,6 @@ public function testI8()
     {
         if (PHP_INT_SIZE == 4 ) {
             $this->markTestSkipped('Can not test i8 as php is compiled in 32 bit mode');
-            return;
         }
 
         $m = $this->newRequest('dummy');

tests/06EncoderTest.php

@@ -60,7 +60,6 @@ public function testLatin15InternalEncoding()
     {
         if (!function_exists('mb_convert_encoding')) {
             $this->markTestSkipped('Miss mbstring extension to test exotic charsets');
-            return;
         }
 
         $string = chr(164);

tests/07ClientTest.php

@@ -3,7 +3,8 @@
 include_once __DIR__ . '/ServerAwareTestCase.php';
 
 /**
- * Tests involving the Client class (and mostly no server).
+ * Tests involving the Client class features (and mostly no server).
+ * @todo review: are there any tests which belong to the ServerTest class?
  */
 class ClientTest extends PhpXmlRpc_ServerAwareTestCase
 {
@@ -40,6 +41,7 @@ public function test404Interop()
         \PhpXmlRpc\PhpXmlRpc::useInteropFaults();
         $r = $this->client->send($m, $this->timeout);
         $this->assertEquals(-32300, $r->faultCode());
+        /// @todo reset this via tear_down
         \PhpXmlRpc\PhpXmlRpc::$xmlrpcerr = $orig;
     }
 
@@ -75,9 +77,8 @@ public function testCurlKAErr()
     {
         if (!function_exists('curl_init')) {
             $this->markTestSkipped('CURL missing: cannot test curl keepalive errors');
-
-            return;
         }
+
         $m = new xmlrpcmsg('examples.stringecho', array(
             new xmlrpcval('hello', 'string'),
         ));
@@ -116,7 +117,8 @@ public function testCustomHeaders($curlOpt)
         $this->assertArrayHasKey('X-Pxr-Test', $ro->scalarVal(), "Testing with curl mode: $curlOpt");
     }
 
-    public function testgetUrl()
+    /// @todo add more permutations, eg. check that PHP_URL_SCHEME is ok with http10, http11, h2 etc...
+    public function testGetUrl()
     {
         $m = $this->client->getUrl(PHP_URL_SCHEME);
         $this->assertEquals($m, $this->client->method);

tests/08ServerTest.php

@@ -52,9 +52,13 @@ public function set_up()
     {
         parent::set_up();
 
+        // these 2 values are injected into the client when calling `send`, and are modified by some tests - reset them
         $this->timeout = 10;
+        $this->method = 'http';
 
         $this->client = $this->getClient();
+
+        /// @todo replace with setOption when dropping the BC layer
         $this->client->request_compression = $this->request_compression;
         $this->client->accepted_compression = $this->accepted_compression;
     }
@@ -75,6 +79,7 @@ public function tear_down()
      */
     protected function send($msg, $errorCode = 0, $returnResponse = false)
     {
+        /// @todo move to injecting timeout and method into `getClient`, use the non-legacy API calling convention
         $r = $this->client->send($msg, $this->timeout, $this->method);
         // for multicall, return directly array of responses
         if (is_array($r)) {
@@ -160,8 +165,8 @@ public function testExoticCharsetsRequests()
         // note that we should disable this call also when mbstring is missing server-side
         if (!function_exists('mb_convert_encoding')) {
             $this->markTestSkipped('Miss mbstring extension to test exotic charsets');
-            return;
         }
+
         $sendString = 'κόσμε'; // Greek word 'kosme'
         $str = '<?xml version="1.0" encoding="_ENC_"?>
 <methodCall>
@@ -192,8 +197,8 @@ public function testExoticCharsetsRequests2()
         // note that we should disable this call also when mbstring is missing server-side
         if (!function_exists('mb_convert_encoding')) {
             $this->markTestSkipped('Miss mbstring extension to test exotic charsets');
-            return;
         }
+
         $sendString = '安室奈美恵'; // Japanese name "Namie Amuro"
         $str = '<?xml version="1.0"?>
 <methodCall>
@@ -219,8 +224,8 @@ public function testExoticCharsetsRequests3()
         // note that we should disable this call also when mbstring is missing server-side
         if (!function_exists('mb_convert_encoding')) {
             $this->markTestSkipped('Miss mbstring extension to test exotic charsets');
-            return;
         }
+
         // the warning suppression is due to utf8_decode being deprecated in php 8.2
         $sendString = @utf8_decode('élève');
         $str = '<?xml version="1.0"?>
@@ -635,7 +640,6 @@ public function testCatchErrors()
         if (version_compare(PHP_VERSION, '7.0.0', '<'))
         {
             $this->markTestSkipped('Cannot test php Error on php < 7.0');
-            return;
         }
 
         // these test for the different server error catching modes