From 757d2a4c02a9ad27ecd2cbdadf683391d8a88621 Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Sun, 20 Nov 2022 13:23:30 +0200 Subject: [PATCH] test/alternator: un-xfail a test which passes on modern Python We had an xfailing test that reproduced a case where Alternator tried to report an error when the request was too long, but the boto library didn't see this error and threw a "Broken Pipe" error instead. It turns out that this wasn't a Scylla bug but rather a bug in urllib3, which overzealously reported a "Broken Pipe" instead of trying to read the server's response. It turns out this issue was already fixed in https://github.com/urllib3/urllib3/pull/1524 and now, on modern installations, the test that used to fail now passes and reports "XPASS". So in this patch we remove the "xfail" tag, and skip the test if running an old version of urllib3. Fixes #8195 Closes #12038 --- test/alternator/test_manual_requests.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/alternator/test_manual_requests.py b/test/alternator/test_manual_requests.py index 4d0132045415..eb1bfb5ce64f 100644 --- a/test/alternator/test_manual_requests.py +++ b/test/alternator/test_manual_requests.py @@ -8,7 +8,9 @@ import pytest import requests import json +import urllib3 from botocore.exceptions import BotoCoreError, ClientError +from packaging.version import Version def gen_json(n): return '{"":'*n + '{}' + '}'*n @@ -100,8 +102,10 @@ def test_too_large_request(dynamodb, test_table): # Content-Length header or chunked encoding (reproduces issue #8196). # 2. The client should be able to recognize this error as a 413 error, not # some I/O error like broken pipe (reproduces issue #8195). -@pytest.mark.xfail(reason="issue #8195, #8196") +@pytest.mark.xfail(reason="issue #8196") def test_too_large_request_chunked(dynamodb, test_table): + if Version(urllib3.__version__) < Version('1.26'): + pytest.skip("urllib3 before 1.26.0 threw broken pipe and did not read response and cause issue #8195. Fixed by pull request urllib3/urllib3#1524") # To make a request very large, we just stuff it with a lot of spaces :-) spaces = ' ' * (17 * 1024 * 1024) req = get_signed_request(dynamodb, 'PutItem', @@ -114,15 +118,16 @@ def generator(s): # request succeeded, and the status_code was 200 instead of 413. assert response.status_code == 413 -@pytest.mark.xfail(reason="issue #8195") def test_too_large_request_content_length(dynamodb, test_table): + if Version(urllib3.__version__) < Version('1.26'): + pytest.skip("urllib3 before 1.26.0 threw broken pipe and did not read response and cause issue #8195. Fixed by pull request urllib3/urllib3#1524") spaces = ' ' * (17 * 1024 * 1024) req = get_signed_request(dynamodb, 'PutItem', '{"TableName": "' + test_table.name + '", ' + spaces + '"Item": {"p": {"S": "x"}, "c": {"S": "x"}}}') response = requests.post(req.url, headers=req.headers, data=req.body, verify=False) # In issue #8195, Alternator closed the connection early, causing the - # library to throw an exception (Broken Pipe) instead noticing the error - # code 413 which the server did send. + # library to incorrectly throw an exception (Broken Pipe) instead noticing + # the error code 413 which the server did send. assert response.status_code == 413 def test_incorrect_json(dynamodb, test_table):