Added a new test file `tests/test_client.py` with comprehensive unit tests for `RPClient` class #237

ipetrushchak · 2024-09-27T11:43:46Z

Added a new test file tests/test_client.py with comprehensive unit tests for RPClient class.
Please assist in resolving the branch creation issue or provide the necessary permissions to proceed.

Add unit tests for client.py

Improve unit tests in elitea_test/test_client.py

Improve unit tests for RPClient

HardNorth

Not LGTM

HardNorth · 2024-10-09T09:12:37Z

elitea_test/test_client.py

+@pytest.fixture
+def rp_client():
+    return RPClient(
+        endpoint="http://example.com",
+        project="test_project",
+        api_key="test_api_key",
+        log_batch_size=20,
+        is_skipped_an_issue=True,
+        verify_ssl=True,
+        retries=3,
+        max_pool_size=50,
+        launch_uuid="test_launch_uuid",
+        http_timeout=(10, 10),
+        log_batch_payload_size=1024,
+        mode="DEFAULT",
+        launch_uuid_print=False,
+        print_output=OutputType.STDOUT
+    )


This fixture looks OK-ish.

HardNorth · 2024-10-09T09:13:30Z

elitea_test/test_client.py

+def test_launch_uuid(rp_client):
+    assert rp_client.launch_uuid == "test_launch_uuid"
+
+def test_endpoint(rp_client):
+    assert rp_client.endpoint == "http://example.com"
+
+def test_project(rp_client):
+    assert rp_client.project == "test_project"
+
+def test_step_reporter(rp_client):
+    assert isinstance(rp_client.step_reporter, StepReporter)
+
+def test_init_session(rp_client):
+    rp_client._RPClient__init_session()
+    assert isinstance(rp_client.session, Session)
+    assert rp_client.session.headers['Authorization'] == 'Bearer test_api_key'


These tests are OK, but have no much value, since it verifies setting class fields in constructor.

HardNorth · 2024-10-09T09:16:56Z

elitea_test/test_client.py

+            has_stats=True,
+            code_ref=None,
+            retry=False,
+            test_case_id=None
+        )
+        assert result == "item_uuid"
+        mock_start_test_item.assert_called_once()
+
+def test_finish_test_item(rp_client):
+    with patch.object(rp_client, 'finish_test_item', return_value="response") as mock_finish_test_item:
+        result = rp_client.finish_test_item(
+            item_id="item_uuid",
+            end_time="2023-01-01T00:00:00.000Z",
+            status="PASSED",
+            issue=None,
+            attributes=[{"key": "value"}],
+            description="Test Description",
+            retry=False
+        )
+        assert result == "response"
+        mock_finish_test_item.assert_called_once()
+
+def test_finish_launch(rp_client):
+    with patch.object(rp_client, 'finish_launch', return_value="response") as mock_finish_launch:
+        result = rp_client.finish_launch(
+            end_time="2023-01-01T00:00:00.000Z",
+            status="PASSED",
+            attributes=[{"key": "value"}]
+        )
+        assert result == "response"
+        mock_finish_launch.assert_called_once()
+
+def test_update_test_item(rp_client):
+    with patch.object(rp_client, 'update_test_item', return_value="response") as mock_update_test_item:
+        result = rp_client.update_test_item(
+            item_uuid="item_uuid",
+            attributes=[{"key": "value"}],
+            description="Updated Description"
+        )
+        assert result == "response"
+        mock_update_test_item.assert_called_once()
+
+def test_get_launch_info(rp_client):
+    with patch.object(rp_client, 'get_launch_info', return_value={"info": "data"}) as mock_get_launch_info:
+        result = rp_client.get_launch_info()
+        assert result == {"info": "data"}
+        mock_get_launch_info.assert_called_once()
+
+def test_get_item_id_by_uuid(rp_client):
+    with patch.object(rp_client, 'get_item_id_by_uuid', return_value="item_id") as mock_get_item_id_by_uuid:
+        result = rp_client.get_item_id_by_uuid("item_uuid")
+        assert result == "item_id"
+        mock_get_item_id_by_uuid.assert_called_once()
+
+def test_get_launch_ui_id(rp_client):
+    with patch.object(rp_client, 'get_launch_ui_id', return_value=123) as mock_get_launch_ui_id:
+        result = rp_client.get_launch_ui_id()
+        assert result == 123
+        mock_get_launch_ui_id.assert_called_once()
+
+def test_get_launch_ui_url(rp_client):
+    with patch.object(rp_client, 'get_launch_ui_url', return_value="http://example.com/launch") as mock_get_launch_ui_url:
+        result = rp_client.get_launch_ui_url()
+        assert result == "http://example.com/launch"
+        mock_get_launch_ui_url.assert_called_once()
+
+def test_get_project_settings(rp_client):
+    with patch.object(rp_client, 'get_project_settings', return_value={"settings": "data"}) as mock_get_project_settings:
+        result = rp_client.get_project_settings()
+        assert result == {"settings": "data"}
+        mock_get_project_settings.assert_called_once()
+
+def test_log(rp_client):
+    with patch.object(rp_client, 'log', return_value=("response",)) as mock_log:
+        result = rp_client.log(
+            time="2023-01-01T00:00:00.000Z",
+            message="Test log message",
+            level="INFO",
+            attachment=None,
+            item_id="item_uuid"
+        )
+        assert result == ("response",)
+        mock_log.assert_called_once()
+
+def test_current_item(rp_client):
+    with patch.object(rp_client, 'current_item', return_value="item_uuid") as mock_current_item:
+        result = rp_client.current_item()
+        assert result == "item_uuid"
+        mock_current_item.assert_called_once()
+
+def test_clone(rp_client):
+    with patch.object(rp_client, 'clone', return_value=rp_client) as mock_clone:
+        result = rp_client.clone()
+        assert result == rp_client
+        mock_clone.assert_called_once()
+
+def test_close(rp_client):
+    with patch.object(rp_client, 'close') as mock_close:
+        rp_client.close()
+        mock_close.assert_called_once()


These tests are pointless at all. It mocks methods and then call this mocked methods, and then verifies that the mock is called. It doesn't even call real code.

HardNorth · 2024-10-09T09:17:32Z

elitea_test/test_client.py

+def test_is_skipped_an_issue(rp_client):
+    assert rp_client.is_skipped_an_issue is True
+
+def test_verify_ssl(rp_client):
+    assert rp_client.verify_ssl is True
+
+def test_retries(rp_client):
+    assert rp_client.retries == 3
+
+def test_max_pool_size(rp_client):
+    assert rp_client.max_pool_size == 50
+
+def test_http_timeout(rp_client):
+    assert rp_client.http_timeout == (10, 10)
+
+def test_log_batch_payload_size(rp_client):
+    assert rp_client.log_batch_payload_size == 1024
+
+def test_mode(rp_client):
+    assert rp_client.mode == "DEFAULT"
+
+def test_launch_uuid_print(rp_client):
+    assert rp_client.launch_uuid_print is False
+
+def test_print_output(rp_client):
+    assert rp_client.print_output == OutputType.STDOUT
+
+def test_log_batch_size(rp_client):
+    assert rp_client.log_batch_size == 20


These tests are OK, but have no much value, since it verifies setting class fields in constructor.

HardNorth · 2024-10-09T09:19:13Z

elitea_test/test_client_updated.py

+@pytest.fixture
+def rp_client():
+    return RPClient(
+        endpoint="http://example.com",
+        project="test_project",
+        api_key="test_api_key",
+        log_batch_size=20,
+        is_skipped_an_issue=True,
+        verify_ssl=True,
+        retries=3,
+        max_pool_size=50,
+        launch_uuid="test_launch_uuid",
+        http_timeout=(10, 10),
+        log_batch_payload_size=1024,
+        mode="DEFAULT",
+        launch_uuid_print=False,
+        print_output=OutputType.STDOUT
+    )


This fixture looks OK-ish.

HardNorth · 2024-10-09T09:20:01Z

elitea_test/test_client_updated.py

+def test_launch_uuid(rp_client):
+    assert rp_client.launch_uuid == "test_launch_uuid"
+
+def test_endpoint(rp_client):
+    assert rp_client.endpoint == "http://example.com"
+
+def test_project(rp_client):
+    assert rp_client.project == "test_project"
+
+def test_step_reporter(rp_client):
+    assert isinstance(rp_client.step_reporter, StepReporter)
+
+def test_init_session(rp_client):
+    rp_client._RPClient__init_session()
+    assert isinstance(rp_client.session, Session)
+    assert rp_client.session.headers['Authorization'] == 'Bearer test_api_key'


These tests are OK, but have no much value, since it verifies setting class fields in constructor.

HardNorth · 2024-10-09T09:21:00Z

elitea_test/test_client_updated.py

+            has_stats=True,
+            code_ref=None,
+            retry=False,
+            test_case_id=None
+        )
+        assert result == "item_uuid"
+        mock_start_test_item.assert_called_once()
+
+def test_finish_test_item(rp_client):
+    with patch.object(rp_client, 'finish_test_item', return_value="response") as mock_finish_test_item:
+        result = rp_client.finish_test_item(
+            item_id="item_uuid",
+            end_time="2023-01-01T00:00:00.000Z",
+            status="PASSED",
+            issue=None,
+            attributes=[{"key": "value"}],
+            description="Test Description",
+            retry=False
+        )
+        assert result == "response"
+        mock_finish_test_item.assert_called_once()
+
+def test_finish_launch(rp_client):
+    with patch.object(rp_client, 'finish_launch', return_value="response") as mock_finish_launch:
+        result = rp_client.finish_launch(
+            end_time="2023-01-01T00:00:00.000Z",
+            status="PASSED",
+            attributes=[{"key": "value"}]
+        )
+        assert result == "response"
+        mock_finish_launch.assert_called_once()
+
+def test_update_test_item(rp_client):
+    with patch.object(rp_client, 'update_test_item', return_value="response") as mock_update_test_item:
+        result = rp_client.update_test_item(
+            item_uuid="item_uuid",
+            attributes=[{"key": "value"}],
+            description="Updated Description"
+        )
+        assert result == "response"
+        mock_update_test_item.assert_called_once()
+
+def test_get_launch_info(rp_client):
+    with patch.object(rp_client, 'get_launch_info', return_value={"info": "data"}) as mock_get_launch_info:
+        result = rp_client.get_launch_info()
+        assert result == {"info": "data"}
+        mock_get_launch_info.assert_called_once()
+
+def test_get_item_id_by_uuid(rp_client):
+    with patch.object(rp_client, 'get_item_id_by_uuid', return_value="item_id") as mock_get_item_id_by_uuid:
+        result = rp_client.get_item_id_by_uuid("item_uuid")
+        assert result == "item_id"
+        mock_get_item_id_by_uuid.assert_called_once()
+
+def test_get_launch_ui_id(rp_client):
+    with patch.object(rp_client, 'get_launch_ui_id', return_value=123) as mock_get_launch_ui_id:
+        result = rp_client.get_launch_ui_id()
+        assert result == 123
+        mock_get_launch_ui_id.assert_called_once()
+
+def test_get_launch_ui_url(rp_client):
+    with patch.object(rp_client, 'get_launch_ui_url', return_value="http://example.com/launch") as mock_get_launch_ui_url:
+        result = rp_client.get_launch_ui_url()
+        assert result == "http://example.com/launch"
+        mock_get_launch_ui_url.assert_called_once()
+
+def test_get_project_settings(rp_client):
+    with patch.object(rp_client, 'get_project_settings', return_value={"settings": "data"}) as mock_get_project_settings:
+        result = rp_client.get_project_settings()
+        assert result == {"settings": "data"}
+        mock_get_project_settings.assert_called_once()
+
+def test_log(rp_client):
+    with patch.object(rp_client, 'log', return_value=("response",)) as mock_log:
+        result = rp_client.log(
+            time="2023-01-01T00:00:00.000Z",
+            message="Test log message",
+            level="INFO",
+            attachment=None,
+            item_id="item_uuid"
+        )
+        assert result == ("response",)
+        mock_log.assert_called_once()
+
+def test_current_item(rp_client):
+    with patch.object(rp_client, 'current_item', return_value="item_uuid") as mock_current_item:
+        result = rp_client.current_item()
+        assert result == "item_uuid"
+        mock_current_item.assert_called_once()
+
+def test_clone(rp_client):
+    with patch.object(rp_client, 'clone', return_value=rp_client) as mock_clone:
+        result = rp_client.clone()
+        assert result == rp_client
+        mock_clone.assert_called_once()
+
+def test_close(rp_client):
+    with patch.object(rp_client, 'close') as mock_close:
+        rp_client.close()
+        mock_close.assert_called_once()


These tests are pointless at all. It mocks methods and then call this mocked methods, and then verifies that the mock is called. It doesn't even call real code.

HardNorth · 2024-10-09T09:22:09Z

elitea_test/test_client_updated.py

+def test_is_skipped_an_issue(rp_client):
+    assert rp_client.is_skipped_an_issue is True
+
+def test_verify_ssl(rp_client):
+    assert rp_client.verify_ssl is True
+
+def test_retries(rp_client):
+    assert rp_client.retries == 3
+
+def test_max_pool_size(rp_client):
+    assert rp_client.max_pool_size == 50
+
+def test_http_timeout(rp_client):
+    assert rp_client.http_timeout == (10, 10)
+
+def test_log_batch_payload_size(rp_client):
+    assert rp_client.log_batch_payload_size == 1024
+
+def test_mode(rp_client):
+    assert rp_client.mode == "DEFAULT"
+
+def test_launch_uuid_print(rp_client):
+    assert rp_client.launch_uuid_print is False
+
+def test_print_output(rp_client):
+    assert rp_client.print_output == OutputType.STDOUT
+
+def test_log_batch_size(rp_client):
+    assert rp_client.log_batch_size == 20


These tests are OK, but have no much value, since it verifies setting class fields in constructor.

HardNorth · 2024-10-09T09:22:57Z

elitea_test/test_client_updated.py

+def test_get_item_id_by_uuid_not_found(rp_client):
+    with patch.object(rp_client, 'get_item_id_by_uuid', return_value=None) as mock_get_item_id_by_uuid:
+        result = rp_client.get_item_id_by_uuid("non_existent_uuid")
+        assert result is None
+        mock_get_item_id_by_uuid.assert_called_once()
+
+def test_start_launch_with_rerun(rp_client):
+    with patch.object(rp_client, 'start_launch', return_value="launch_uuid") as mock_start_launch:
+        result = rp_client.start_launch(
+            name="Test Launch",
+            start_time="2023-01-01T00:00:00.000Z",
+            description="Test Description",
+            attributes=[{"key": "value"}],
+            rerun=True,
+            rerun_of="previous_launch_uuid"
+        )
+        assert result == "launch_uuid"
+        mock_start_launch.assert_called_once()
+
+def test_finish_test_item_with_issue(rp_client):
+    issue = Issue(issue_type="BUG", comment="Test issue")
+    with patch.object(rp_client, 'finish_test_item', return_value="response") as mock_finish_test_item:
+        result = rp_client.finish_test_item(
+            item_id="item_uuid",
+            end_time="2023-01-01T00:00:00.000Z",
+            status="FAILED",
+            issue=issue,
+            attributes=[{"key": "value"}],
+            description="Test Description",
+            retry=False
+        )
+        assert result == "response"
+        mock_finish_test_item.assert_called_once()
+
+def test_log_with_attachment(rp_client):
+    attachment = {"name": "screenshot.png", "data": b"binary_data", "mime": "image/png"}
+    with patch.object(rp_client, 'log', return_value=("response",)) as mock_log:
+        result = rp_client.log(
+            time="2023-01-01T00:00:00.000Z",
+            message="Test log message with attachment",
+            level="INFO",
+            attachment=attachment,
+            item_id="item_uuid"
+        )
+        assert result == ("response",)
+        mock_log.assert_called_once()


These tests are pointless at all. It mocks methods and then call this mocked methods, and then verifies that the mock is called. It doesn't even call real code.

oleksiidonchakkf

Did a quick glance over the code, feel free to ignore :)

oleksiidonchakkf · 2024-10-10T11:08:20Z

elitea_test/test_client.py

+    )
+
+def test_launch_uuid(rp_client):
+    assert rp_client.launch_uuid == "test_launch_uuid"


I would rather make a single test with no fixtures:

def test_constructor(): rp_client = ... assert rp_client.field1 == "..." assert rp_client.field2 == "..."

oleksiidonchakkf · 2024-10-10T11:09:28Z

elitea_test/test_client.py

+    assert isinstance(rp_client.step_reporter, StepReporter)
+
+def test_init_session(rp_client):
+    rp_client._RPClient__init_session()


This line is calling a private method. Sign of something wrong with design

oleksiidonchakkf · 2024-10-10T11:12:53Z

elitea_test/test_client.py

+
+def test_get_launch_info(rp_client):
+    with patch.object(rp_client, 'get_launch_info', return_value={"info": "data"}) as mock_get_launch_info:
+        result = rp_client.get_launch_info()


Tests like these don't make much sense because they are doing the following:

Create an object

Mock its function to return a value

Call the function

Assert it has been called and returned the value (Sure thing! What would it return otherwise?)

oleksiidonchakkf · 2024-10-10T11:15:53Z

elitea_test/test_client_updated.py

+        assert result == "item_id"
+        mock_get_item_id_by_uuid.assert_called_once()
+
+def test_get_launch_ui_id(rp_client):


Same for this file. The only meaningful test is the one testing the constructor

By the way, the idea of creating an object via a fixture is ok for the tests if you need a fresh object per test. So, the code is correct

Add Integrational Tests

Add Integrational Tests for RPClient

new commit and PR

4225ad1

HardNorth closed this Sep 27, 2024

ipetrushchak added 7 commits September 30, 2024 15:00

Create elitea_test/test_client.py

1bcc59c

Merge pull request #1 from ipetrushchak/add-unit-tests-for-client_v1

153dee1

Add unit tests for client.py

Update elitea_test/test_client.py

f4afca7

Merge pull request #2 from ipetrushchak/improve-unit-tests-for-client

fe41079

Improve unit tests in elitea_test/test_client.py

Create elitea_test/test_client_updated.py

fb28175

Merge pull request #3 from ipetrushchak/improve-unit-tests-for-client_v2

048b3d4

Improve unit tests for RPClient

Delete tests_elitea directory

bd81e27

HardNorth reopened this Oct 9, 2024

HardNorth requested changes Oct 9, 2024

View reviewed changes

oleksiidonchakkf reviewed Oct 10, 2024

View reviewed changes

ipetrushchak added 4 commits October 30, 2024 16:24

Update elitea_test/test_client.py

08d1b60

Update elitea_test/test_client.py

b6e7cf0

Merge pull request #5 from ipetrushchak/add-integrational-tests_v2

9918163

Add Integrational Tests

Merge pull request #4 from ipetrushchak/add-integrational-tests

879b0a0

Add Integrational Tests for RPClient

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a new test file `tests/test_client.py` with comprehensive unit tests for `RPClient` class #237

Added a new test file `tests/test_client.py` with comprehensive unit tests for `RPClient` class #237

ipetrushchak commented Sep 27, 2024

HardNorth left a comment

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

HardNorth Oct 9, 2024

oleksiidonchakkf left a comment

oleksiidonchakkf Oct 10, 2024 •

edited

Loading

oleksiidonchakkf Oct 10, 2024

oleksiidonchakkf Oct 10, 2024

oleksiidonchakkf Oct 10, 2024

Added a new test file tests/test_client.py with comprehensive unit tests for RPClient class #237

Are you sure you want to change the base?

Added a new test file tests/test_client.py with comprehensive unit tests for RPClient class #237

Conversation

ipetrushchak commented Sep 27, 2024

HardNorth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleksiidonchakkf left a comment

Choose a reason for hiding this comment

oleksiidonchakkf Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Added a new test file `tests/test_client.py` with comprehensive unit tests for `RPClient` class #237

Added a new test file `tests/test_client.py` with comprehensive unit tests for `RPClient` class #237

oleksiidonchakkf Oct 10, 2024 •

edited

Loading