Skip to content

Commit

Permalink
Fix various problems in the pbench_results_push tests
Browse files Browse the repository at this point in the history 
Fixes distributed-system-analysis#3374

This is a continuation of distributed-system-analysis#3348, implementing fixes to the
pbench_results_push tests. The main one is to subsume some exception
tests under the parametrized "normal" case and eliminate redundant
tests.
  • Loading branch information
@ndokos
ndokos committed Apr 12, 2023
1 parent e0dc06d commit dcc1ee3
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 147 deletions.
2 changes: 0 additions & 2 deletions lib/pbench/test/unit/agent/task/test_copy_result_tb.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ def request_callback(request: requests.Request):
res = crt.copy_result_tb("token", access)

assert res.status_code == HTTPStatus.CREATED
# If we got this far without an exception, then the test passes.

@responses.activate
@pytest.mark.parametrize(
Expand Down Expand Up @@ -143,7 +142,6 @@ def request_callback(request: requests.Request):
res = crt.copy_result_tb("token", access, metadata)

assert res.status_code == HTTPStatus.CREATED
# If we got this far without an exception, then the test passes.

@responses.activate
def test_connection_error(self, monkeypatch, agent_logger):
Expand Down
239 changes: 94 additions & 145 deletions lib/pbench/test/unit/agent/task/test_results_push.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from http import HTTPStatus
import logging
import os
from typing import Dict, Optional, Union

Expand All @@ -26,15 +25,16 @@ class TestResultsPush:

@staticmethod
def add_http_mock_response(
status_code: HTTPStatus = None, message: Optional[Union[str, Dict]] = None
status_code: HTTPStatus = HTTPStatus.CREATED,
message: Optional[Union[str, Dict]] = None,
):
parms = {}
if status_code:
parms["status"] = status_code

if isinstance(message, dict):
parms["json"] = message
elif isinstance(message, str):
elif isinstance(message, (str, Exception)):
parms["body"] = message

responses.add(
Expand All @@ -43,18 +43,6 @@ def add_http_mock_response(
**parms,
)

@staticmethod
def add_connectionerr_mock_response():
responses.add(
responses.PUT,
f"{TestResultsPush.URL}/upload/{os.path.basename(tarball)}",
body=requests.exceptions.ConnectionError(
"<urllib3.connection.HTTPConnection object at 0x1080854c0>: "
"Failed to establish a new connection: [Errno 8] "
"nodename nor servname provided, or not known"
),
)

@staticmethod
@responses.activate
def test_help():
Expand Down Expand Up @@ -101,7 +89,6 @@ def test_bad_arg():
def test_meta_args():
"""Test operation when irregular arguments and options are specified"""

TestResultsPush.add_http_mock_response()
runner = CliRunner(mix_stderr=False)
result = runner.invoke(
main,
Expand Down Expand Up @@ -162,46 +149,105 @@ def test_multiple_meta_args_single_option():

@staticmethod
@responses.activate
def test_multiple_meta_args():
"""Test normal operation when all arguments and options are specified"""
def test_token_omitted():
"""Test error when the token option is omitted"""

runner = CliRunner(mix_stderr=False)
result = runner.invoke(main, args=[tarball])
assert result.exit_code == 2, result.stderr
assert "Missing option '--token'" in str(result.stderr)

@staticmethod
@responses.activate
def test_token_envar(monkeypatch):
"""Test normal operation with the token in an environment variable"""

monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
TestResultsPush.add_http_mock_response()
runner = CliRunner(mix_stderr=False)
result = runner.invoke(main, args=[tarball])
assert result.exit_code == 0, result.stderr
assert result.stdout == ""

@staticmethod
@responses.activate
def test_access_error(monkeypatch):
"""Test error in access value"""

monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
runner = CliRunner(mix_stderr=False)
result = runner.invoke(
main,
args=[
TestResultsPush.TOKN_SWITCH,
TestResultsPush.TOKN_TEXT,
TestResultsPush.ACCESS_SWITCH,
TestResultsPush.ACCESS_TEXT,
TestResultsPush.META_SWITCH,
TestResultsPush.META_TEXT_TEST,
TestResultsPush.META_SWITCH,
TestResultsPush.META_TEXT_FOO,
tarball,
TestResultsPush.ACCESS_SWITCH,
TestResultsPush.ACCESS_WRONG_TEXT,
],
)
assert result.exit_code == 0, result.stderr
assert result.stdout == ""
assert result.exit_code == 2, result.stderr
assert "Error: Invalid value for '-a' / '--access': 'public/private'" in str(
result.stderr
)

@staticmethod
@responses.activate
@pytest.mark.parametrize(
"status_code,message,exit_code",
(
(HTTPStatus.CREATED, None, 0),
(HTTPStatus.OK, {"message": "Dup"}, 0),
(HTTPStatus.OK, "Dup", 0),
(HTTPStatus.NO_CONTENT, {"message": "No content"}, 0),
(HTTPStatus.NO_CONTENT, "No content", 0),
(
HTTPStatus.CREATED,
None,
0,
),
(
HTTPStatus.OK,
{"message": "Dup"},
0,
),
(
HTTPStatus.OK,
"Dup",
0,
),
(
HTTPStatus.NO_CONTENT,
{"message": "No content"},
0,
),
(
HTTPStatus.NO_CONTENT,
"No content",
0,
),
(
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
{"message": "Request Entity Too Large"},
1,
),
(HTTPStatus.REQUEST_ENTITY_TOO_LARGE, "Request Entity Too Large", 1),
(HTTPStatus.NOT_FOUND, {"message": "Not Found"}, 1),
(HTTPStatus.NOT_FOUND, "Not Found", 1),
(
HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
"Request Entity Too Large",
1,
),
(
HTTPStatus.NOT_FOUND,
{"message": "Not Found"},
1,
),
(
HTTPStatus.NOT_FOUND,
"Not Found",
1,
),
(
0,
requests.exceptions.ConnectionError(
"<urllib3.connection.HTTPConnection object at 0x1080854c0>: "
"Failed to establish a new connection: [Errno 8] "
"nodename nor servname provided, or not known"
),
1,
),
),
)
def test_push_status(status_code, message, exit_code):
Expand All @@ -221,118 +267,21 @@ def test_push_status(status_code, message, exit_code):
tarball,
],
)

assert result.exit_code == exit_code, result.stderr
assert result.stdout == ""

try:
err_msg = "" if not message else message["message"]
except TypeError:
if not message:
err_msg = ""
elif isinstance(message, dict):
err_msg = message["message"]
elif isinstance(message, str):
err_msg = message
elif isinstance(message, Exception):
err_msg = str(message)
else:
assert False, "message must be dict, string, Exception or None"

if status_code >= HTTPStatus.BAD_REQUEST:
if status_code >= 400:
err_msg = f"HTTP Error status: {status_code.value}, message: {err_msg}"
assert result.stderr.strip() == err_msg

@staticmethod
@responses.activate
def test_error_too_large_tarball():
"""Test normal operation when all arguments and options are specified"""

TestResultsPush.add_http_mock_response(
status_code=HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
message={"message": "Request Entity Too Large"},
)
runner = CliRunner(mix_stderr=False)
result = runner.invoke(
main,
args=[
TestResultsPush.TOKN_SWITCH,
TestResultsPush.TOKN_TEXT,
TestResultsPush.ACCESS_SWITCH,
TestResultsPush.ACCESS_TEXT,
TestResultsPush.META_SWITCH,
TestResultsPush.META_TEXT_TEST + "," + TestResultsPush.META_TEXT_FOO,
tarball,
],
)
assert result.exit_code == 1, result.stderr
assert (
result.stderr
== "HTTP Error status: 413, message: Request Entity Too Large\n"
)

@staticmethod
@responses.activate
def test_token_omitted():
"""Test error when the token option is omitted"""

TestResultsPush.add_http_mock_response()
runner = CliRunner(mix_stderr=False)
result = runner.invoke(main, args=[tarball])
assert result.exit_code == 2, result.stderr
assert "Missing option '--token'" in str(result.stderr)

@staticmethod
@responses.activate
def test_token_envar(monkeypatch, caplog):
"""Test normal operation with the token in an environment variable"""

monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
TestResultsPush.add_http_mock_response()
caplog.set_level(logging.DEBUG)
runner = CliRunner(mix_stderr=False)
result = runner.invoke(main, args=[tarball])
assert result.exit_code == 0, result.stderr
assert result.stdout == ""

@staticmethod
@responses.activate
def test_access_error(monkeypatch, caplog):
"""Test error in access value"""

monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
TestResultsPush.add_http_mock_response()
caplog.set_level(logging.DEBUG)
runner = CliRunner(mix_stderr=False)
result = runner.invoke(
main,
args=[
tarball,
TestResultsPush.ACCESS_SWITCH,
TestResultsPush.ACCESS_WRONG_TEXT,
],
)
assert result.exit_code == 2, result.stderr
assert "Error: Invalid value for '-a' / '--access': 'public/private'" in str(
result.stderr
)

@staticmethod
@responses.activate
def test_connection_error(monkeypatch, caplog):
"""Test handling of connection errors"""

monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
TestResultsPush.add_connectionerr_mock_response()
caplog.set_level(logging.DEBUG)
runner = CliRunner(mix_stderr=False)
result = runner.invoke(main, args=[tarball])
assert result.exit_code == 1
assert str(result.stderr).startswith("Cannot connect to")

@staticmethod
@responses.activate
def test_http_error(monkeypatch, caplog):
"""Test handling of 404 errors"""

monkeypatch.setenv("PBENCH_ACCESS_TOKEN", TestResultsPush.TOKN_TEXT)
TestResultsPush.add_http_mock_response(
HTTPStatus.NOT_FOUND, message={"message": "Not Found"}
)
caplog.set_level(logging.DEBUG)
runner = CliRunner(mix_stderr=False)
result = runner.invoke(main, args=[tarball])
assert result.exit_code == 1
assert (
"Not Found" in result.stderr
), f"stderr: {result.stderr!r}; stdout: {result.stdout!r}"
assert err_msg in result.stderr.strip()

0 comments on commit dcc1ee3

Please sign in to comment.