Browse Source

pytest: improvements

- set CURL_CI for pytest runs in CI environments
- exclude timing sensitive tests from CI runs
- for failed results, list only the log and stat of
  the failed transfer

- fix type in http.c comment

Closes #11812
Stefan Eissing 8 months ago
parent
commit
3b30cc1a0d

+ 2 - 1
.github/workflows/linux.yml

@@ -404,7 +404,8 @@ jobs:
       # run for `tests` directory, so pytest does not pick up any other
       # packages we might have built here
       run:
-        pytest -v tests
+        pytest tests
       name: 'run pytest'
       env:
         TFLAGS: "${{ matrix.build.tflags }}"
+        CURL_CI: github

+ 4 - 2
.github/workflows/ngtcp2-linux.yml

@@ -263,15 +263,17 @@ jobs:
       env:
         TFLAGS: "${{ matrix.build.tflags }}"
 
-    - run: pytest -v
+    - run: pytest tests
       name: 'run pytest'
       env:
         TFLAGS: "${{ matrix.build.tflags }}"
+        CURL_CI: github
 
-    - run: pytest
+    - run: pytest tests
       name: 'run pytest with slowed network'
       env:
         # 33% of sends are EAGAINed
         CURL_DBG_SOCK_WBLOCK: 33
         # only 80% of data > 10 bytes is send
         CURL_DBG_SOCK_WPARTIAL: 80
+        CURL_CI: github

+ 2 - 1
.github/workflows/quiche-linux.yml

@@ -202,7 +202,8 @@ jobs:
       env:
         TFLAGS: "${{ matrix.build.tflags }}"
 
-    - run: pytest -v tests/http
+    - run: pytest tests
       name: 'run pytest'
       env:
         TFLAGS: "${{ matrix.build.tflags }}"
+        CURL_CI: github

+ 1 - 1
lib/http.c

@@ -2707,7 +2707,7 @@ CURLcode Curl_http_bodysend(struct Curl_easy *data, struct connectdata *conn,
 
         if(!data->req.upload_chunky) {
           /* We're not sending it 'chunked', append it to the request
-             already now to reduce the number if send() calls */
+             already now to reduce the number of send() calls */
           result = Curl_dyn_addn(r, data->set.postfields,
                                  (size_t)http->postsize);
           included_body = http->postsize;

+ 3 - 0
tests/http/test_02_download.py

@@ -201,6 +201,7 @@ class TestDownload:
         r.check_response(count=count, http_status=200)
 
     @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
+    @pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
     @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
     def test_02_10_10MB_serial(self, env: Env,
                               httpd, nghttpx, repeat, proto):
@@ -213,6 +214,7 @@ class TestDownload:
         r.check_response(count=count, http_status=200)
 
     @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
+    @pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
     @pytest.mark.parametrize("proto", ['h2', 'h3'])
     def test_02_11_10MB_parallel(self, env: Env,
                               httpd, nghttpx, repeat, proto):
@@ -255,6 +257,7 @@ class TestDownload:
         r.check_response(count=count, http_status=200)
 
     @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
+    @pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
     def test_02_20_h2_small_frames(self, env: Env, httpd, repeat):
         # Test case to reproduce content corruption as observed in
         # https://github.com/curl/curl/issues/10525

+ 1 - 0
tests/http/test_04_stuttered.py

@@ -36,6 +36,7 @@ log = logging.getLogger(__name__)
 
 
 @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
+@pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
 class TestStuttered:
 
     @pytest.fixture(autouse=True, scope='class')

+ 3 - 0
tests/http/test_08_caddy.py

@@ -111,6 +111,7 @@ class TestCaddy:
 
     # download 5MB files sequentially
     @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
+    @pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
     @pytest.mark.parametrize("proto", ['h2', 'h3'])
     def test_08_04a_download_10mb_sequential(self, env: Env, caddy: Caddy,
                                            repeat, proto):
@@ -126,6 +127,7 @@ class TestCaddy:
 
     # download 10MB files sequentially
     @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
+    @pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
     @pytest.mark.parametrize("proto", ['h2', 'h3'])
     def test_08_04b_download_10mb_sequential(self, env: Env, caddy: Caddy,
                                            repeat, proto):
@@ -142,6 +144,7 @@ class TestCaddy:
     # download 10MB files parallel
     @pytest.mark.skipif(condition=Env().slow_network, reason="not suitable for slow network tests")
     @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
+    @pytest.mark.skipif(condition=Env().ci_run, reason="not suitable for CI runs")
     def test_08_05_download_1mb_parallel(self, env: Env, caddy: Caddy,
                                          repeat, proto):
         if proto == 'h3' and not env.have_h3_curl():

+ 27 - 10
tests/http/testenv/curl.py

@@ -24,7 +24,6 @@
 #
 ###########################################################################
 #
-import pytest
 import json
 import logging
 import os
@@ -199,15 +198,15 @@ class ExecResult:
             if self.with_stats:
                 for idx, x in enumerate(self.stats):
                     assert 'http_code' in x, \
-                        f'response #{idx} reports no http_code\n{self.dump_logs()}'
+                        f'response #{idx} reports no http_code\n{self.dump_stat(x)}'
                     assert x['http_code'] == http_status, \
                         f'response #{idx} http_code: expected {http_status}, '\
-                        f'got {x["http_code"]}\n{self.dump_logs()}'
+                        f'got {x["http_code"]}\n{self.dump_stat(x)}'
             else:
                 for idx, x in enumerate(self.responses):
                     assert x['status'] == http_status, \
                         f'response #{idx} status: expected {http_status},'\
-                        f'got {x["status"]}\n{self.dump_logs()}'
+                        f'got {x["status"]}\n{self.dump_stat(x)}'
         if protocol is not None:
             if self.with_stats:
                 http_version = None
@@ -221,7 +220,7 @@ class ExecResult:
                     for idx, x in enumerate(self.stats):
                         assert x['http_version'] == http_version, \
                             f'response #{idx} protocol: expected http/{http_version},' \
-                            f'got version {x["http_version"]}\n{self.dump_logs()}'
+                            f'got version {x["http_version"]}\n{self.dump_stat(x)}'
             else:
                 for idx, x in enumerate(self.responses):
                     assert x['protocol'] == protocol, \
@@ -241,26 +240,44 @@ class ExecResult:
         if http_status is not None:
             for idx, x in enumerate(self.stats):
                 assert 'http_code' in x, \
-                    f'status #{idx} reports no http_code\n{self.dump_logs()}'
+                    f'status #{idx} reports no http_code\n{self.dump_stat(x)}'
                 assert x['http_code'] == http_status, \
                     f'status #{idx} http_code: expected {http_status}, '\
-                    f'got {x["http_code"]}\n{self.dump_logs()}'
+                    f'got {x["http_code"]}\n{self.dump_stat(x)}'
         if exitcode is not None:
             for idx, x in enumerate(self.stats):
                 if 'exitcode' in x:
                     assert x['exitcode'] == 0, \
                         f'status #{idx} exitcode: expected {exitcode}, '\
-                        f'got {x["exitcode"]}\n{self.dump_logs()}'
+                        f'got {x["exitcode"]}\n{self.dump_stat(x)}'
 
     def dump_logs(self):
-        lines = []
-        lines.append('>>--stdout ----------------------------------------------\n')
+        lines = ['>>--stdout ----------------------------------------------\n']
         lines.extend(self._stdout)
         lines.append('>>--stderr ----------------------------------------------\n')
         lines.extend(self._stderr)
         lines.append('<<-------------------------------------------------------\n')
         return ''.join(lines)
 
+    def dump_stat(self, x):
+        lines = [
+            'json stat from curl:',
+            json.JSONEncoder(indent=2).encode(x),
+        ]
+        if 'xfer_id' in x:
+            xfer_id = x['xfer_id']
+            lines.append(f'>>--xfer {xfer_id} trace:\n')
+            lines.extend(self.xfer_trace_for(xfer_id))
+        else:
+            lines.append('>>--full trace-------------------------------------------\n')
+            lines.extend(self._stderr)
+            lines.append('<<-------------------------------------------------------\n')
+        return ''.join(lines)
+
+    def xfer_trace_for(self, xfer_id) -> List[str]:
+            pat = re.compile(f'^[^[]* \\[{xfer_id}-.*$')
+            return [line for line in self._stderr if pat.match(line)]
+
 
 class CurlClient:
 

+ 4 - 0
tests/http/testenv/env.py

@@ -444,6 +444,10 @@ class Env:
         return "CURL_DBG_SOCK_WBLOCK" in os.environ or \
                "CURL_DBG_SOCK_WPARTIAL" in os.environ
 
+    @property
+    def ci_run(self) -> bool:
+        return "CURL_CI" in os.environ
+
     def authority_for(self, domain: str, alpn_proto: Optional[str] = None):
         if alpn_proto is None or \
                 alpn_proto in ['h2', 'http/1.1', 'http/1.0', 'http/0.9']: