Enhance tests for client [WIP] #239
Conversation
| from reportportal_client.core.rp_issues import Issue | ||
| from reportportal_client.errors import RPInvalidStepUsage, RetentionError | ||
| from reportportal_client.steps import step | ||
|
|
There was a problem hiding this comment.
some methods are not imported
| from reportportal_client.client import RP, ProjectSettings | ||
| from reportportal_client.core.rp_issues import Issue | ||
| from reportportal_client.core.rp_requests import RPFile | ||
| from tests import REPORT_ENDPOINT_V1, ROOT_URI, PROJECT_NAME, LAUNCH_ID, NO_OP |
There was a problem hiding this comment.
not sure where these imports came from, can't find these in the tests/init.py
| rp.log_batch_size = 2 | ||
| rp.log_batch_payload_size = 3 | ||
| result = rp.log(None, None, 'TEST') | ||
| assert result |
There was a problem hiding this comment.
Should it check also something else but assert result?
| from reportportal_client.aio.client import TaskInfo | ||
| from reportportal_client.aio.client import DEFAULT_TASK_TIMEOUT | ||
| from reportportal_client.aio.client import DEFAULT_SHUTDOWN_TASK_TIMEOUT | ||
| from reportportal_client.aio.multiplexer import Multiplexer |
There was a problem hiding this comment.
Which Multiplexer? I can't find any in the related module
| from tests import REPORT_ENDPOINT_V1, ROOT_URI, PROJECT_NAME, LAUNCH_ID, NO_OP | ||
|
|
||
|
|
||
| def test_mp_init(): |
There was a problem hiding this comment.
This test as a whole looks strange:
- Create a mock locally (even though the line itself looks incorrect, mocking usually is done in a different way, see https://docs.python.org/3/library/unittest.mock.html
- Create a multiplexer
- Check its private (!) variable to be the mock. Check by reference
- Create an instance of a non-imported class
- Assert something again
| assert rp.shutdown.called | ||
|
|
||
|
|
||
| class RpChild(Multiplexer): |
There was a problem hiding this comment.
Class created in the middle of a test suite
These things "shutdown", "add_child" and "remove_child" are static variables when declared like this, if I'm not mistaken
| mock_loop.sock_coro = NO_OP | ||
| task = Task(0.0001, NO_OP) | ||
| result = yield from task.execute(mock_loop) | ||
| assert result |
There was a problem hiding this comment.
No mentions of any "retries" here
|
|
||
| def test_task_info(): | ||
| ti = TaskInfo(0, 1, 2, 3) | ||
| assert ti.attempt is 0 |
There was a problem hiding this comment.
I would rather not write tests for a data class
By the way, it's better to compare by value (==), not by reference (is)
|
|
||
| @pytest.mark.asyncio | ||
| async def test_aio_init_default_values(): | ||
| client = RetryingClient('https://endpoint') |
There was a problem hiding this comment.
This class doesn't exist at all. However, the tests are better than in the file 1
At least, I can't find anything obviously wrong without looking into the implementation
| def test_client_init_verify_ignore_connectivity_true(mock_session): | ||
| client = Client('http://endpoint', 'project', 'api_key', | ||
| verify_ssl=False, ignore_connectivity=True) | ||
| expect(lambda: assert_that(client.session, has_properties({'verify': False}))) |
There was a problem hiding this comment.
Here a different framework is used
From what I can see, for tests like this there is no need for any delayed asserts
Just "assert client.session.verify == False" and that's it
| def test_client_init_verify_ignore_connectivity_false(mock_session): | ||
| client = Client('http://endpoint', 'project', 'api_key', | ||
| verify_ssl=False, ignore_connectivity=False) | ||
| expect(contains('[DEFAULT]\ndisable_warnings = True\nverify_ssl = False')).one_of( |
There was a problem hiding this comment.
I have no idea what these test methods come from
If from https://github.com/pr4bh4sh/delayed-assert than I thought it contained only "expect"...
The machine is clearly hallucinating with ".empty()", ".ending_with()" and their connections
"str" and "map" are python methods that definitely mean something different to what the machine were trying to say
|
|
||
| @mock.patch('reportportal_client.aio.client.requests.Session') | ||
| def test_rp_client_start_with_overriding_launch_attributes(self, mock_session): | ||
| rp_truncation_msg = 'Attribute values cannot exceed 128 characters in length. Value got: veryverylongattributevalue' |
There was a problem hiding this comment.
This value "veryverylongattributevalue", it's definitely for shorter than 128 characters
| @mock.patch('reportportal_client.aio.client.requests.Session') | ||
| def test_rp_client_set_attributes_without_values(self, mock_session): | ||
| mock_finish = mock_session().post | ||
| mock_finish.return_value = mock.Mock(json=lambda: {}, status_code=200) |
There was a problem hiding this comment.
A lot of lines like these. Need to collapse them to a function
As for the lines that do testing, they look better but I can't tell you if they are correct without launching them first
The exercise is done within the experiment of AI unittest generation
Unitttest generated for the client.py https://github.com/reportportal/client-Python/blob/develop/reportportal_client/client.py