From 95c6edeaa3b57fa6ba5c4a86da837709eb8a6ef0 Mon Sep 17 00:00:00 2001 From: lleeoo Date: Thu, 12 Oct 2023 12:09:04 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8(tests)=20clean=20tests=20by=20fact?= =?UTF-8?q?oring=20statements=20and=20renaming?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mocking statements tests now has its own function, to avoid duplicating large dict objects. Also, tests now have shorter names for clarity. --- tests/api/test_statements_get.py | 78 +++++----- tests/api/test_statements_post.py | 206 ++++---------------------- tests/api/test_statements_put.py | 187 ++++------------------- tests/backends/http/test_async_lrs.py | 61 +++----- tests/fixtures/auth.py | 4 +- tests/helpers.py | 93 ++++++++++-- tests/test_helpers.py | 61 ++++++++ 7 files changed, 259 insertions(+), 431 deletions(-) diff --git a/tests/api/test_statements_get.py b/tests/api/test_statements_get.py index 9aa0ff742..56cdd2baf 100644 --- a/tests/api/test_statements_get.py +++ b/tests/api/test_statements_get.py @@ -28,8 +28,8 @@ get_mongo_test_backend, ) -from ..fixtures.auth import create_user -from ..helpers import create_mock_activity, create_mock_agent +from ..fixtures.auth import mock_basic_auth_user +from ..helpers import mock_activity, mock_agent client = TestClient(app) @@ -117,7 +117,7 @@ def _insert_statements_and_monkeypatch_backend(statements): "account_different_home_page", ], ) -def test_api_statements_get_statements_mine( +def test_api_statements_get_mine( monkeypatch, fs, insert_statements_and_monkeypatch_backend, ifi ): """(Security) Test that the get statements API route, given a "mine=True" @@ -128,27 +128,29 @@ def test_api_statements_get_statements_mine( # Create two distinct agents if ifi == "account_same_home_page": - agent_1 = create_mock_agent("account", 1, home_page_id=1) - agent_1_bis = create_mock_agent( + agent_1 = mock_agent("account", 1, home_page_id=1) + agent_1_bis = mock_agent( "account", 1, home_page_id=1, name="name", use_object_type=False ) - agent_2 = create_mock_agent("account", 2, home_page_id=1) + agent_2 = mock_agent("account", 2, home_page_id=1) elif ifi == "account_different_home_page": - agent_1 = create_mock_agent("account", 1, home_page_id=1) - agent_1_bis = create_mock_agent( + agent_1 = mock_agent("account", 1, home_page_id=1) + agent_1_bis = mock_agent( "account", 1, home_page_id=1, name="name", use_object_type=False ) - agent_2 = create_mock_agent("account", 1, home_page_id=2) + agent_2 = mock_agent("account", 1, home_page_id=2) else: - agent_1 = create_mock_agent(ifi, 1) - agent_1_bis = create_mock_agent(ifi, 1, name="name", use_object_type=False) - agent_2 = create_mock_agent(ifi, 2) + agent_1 = mock_agent(ifi, 1) + agent_1_bis = mock_agent(ifi, 1, name="name", use_object_type=False) + agent_2 = mock_agent(ifi, 2) username_1 = "jane" password_1 = "janepwd" scopes = [] - credentials_1_bis = create_user(fs, username_1, password_1, scopes, agent_1_bis) + credentials_1_bis = mock_basic_auth_user( + fs, username_1, password_1, scopes, agent_1_bis + ) # Clear cache before each test iteration get_authenticated_user.cache_clear() @@ -230,7 +232,7 @@ def test_api_statements_get_statements_mine( assert response.status_code == 422 -def test_api_statements_get_statements( +def test_api_statements_get( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route without any filters set up.""" @@ -258,7 +260,7 @@ def test_api_statements_get_statements( assert response.json() == {"statements": [statements[1], statements[0]]} -def test_api_statements_get_statements_ascending( +def test_api_statements_get_ascending( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given an "ascending" query parameter, should @@ -287,7 +289,7 @@ def test_api_statements_get_statements_ascending( assert response.json() == {"statements": [statements[0], statements[1]]} -def test_api_statements_get_statements_by_statement_id( +def test_api_statements_get_by_statement_id( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given a "statementId" query parameter, should @@ -326,7 +328,7 @@ def test_api_statements_get_statements_by_statement_id( "account_different_home_page", ], ) -def test_api_statements_get_statements_by_agent( +def test_api_statements_get_by_agent( ifi, insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given an "agent" query parameter, should @@ -336,14 +338,14 @@ def test_api_statements_get_statements_by_agent( # Create two distinct agents if ifi == "account_same_home_page": - agent_1 = create_mock_agent("account", 1, home_page_id=1) - agent_2 = create_mock_agent("account", 2, home_page_id=1) + agent_1 = mock_agent("account", 1, home_page_id=1) + agent_2 = mock_agent("account", 2, home_page_id=1) elif ifi == "account_different_home_page": - agent_1 = create_mock_agent("account", 1, home_page_id=1) - agent_2 = create_mock_agent("account", 1, home_page_id=2) + agent_1 = mock_agent("account", 1, home_page_id=1) + agent_2 = mock_agent("account", 1, home_page_id=2) else: - agent_1 = create_mock_agent(ifi, 1) - agent_2 = create_mock_agent(ifi, 2) + agent_1 = mock_agent(ifi, 1) + agent_2 = mock_agent(ifi, 2) statements = [ { @@ -370,7 +372,7 @@ def test_api_statements_get_statements_by_agent( assert response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_by_verb( +def test_api_statements_get_by_verb( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given a "verb" query parameter, should @@ -401,7 +403,7 @@ def test_api_statements_get_statements_by_verb( assert response.json() == {"statements": [statements[1]]} -def test_api_statements_get_statements_by_activity( +def test_api_statements_get_by_activity( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given an "activity" query parameter, should @@ -409,8 +411,8 @@ def test_api_statements_get_statements_by_activity( """ # pylint: disable=redefined-outer-name - activity_0 = create_mock_activity(0) - activity_1 = create_mock_activity(1) + activity_0 = mock_activity(0) + activity_1 = mock_activity(1) statements = [ { @@ -444,7 +446,7 @@ def test_api_statements_get_statements_by_activity( assert response.json()["detail"][0]["msg"] == "'INVALID_IRI' is not a valid 'IRI'." -def test_api_statements_get_statements_since_timestamp( +def test_api_statements_get_since_timestamp( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given a "since" query parameter, should @@ -474,7 +476,7 @@ def test_api_statements_get_statements_since_timestamp( assert response.json() == {"statements": [statements[1]]} -def test_api_statements_get_statements_until_timestamp( +def test_api_statements_get_until_timestamp( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given an "until" query parameter, @@ -504,7 +506,7 @@ def test_api_statements_get_statements_until_timestamp( assert response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_with_pagination( +def test_api_statements_get_with_pagination( monkeypatch, insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given a request leading to more results than @@ -574,7 +576,7 @@ def test_api_statements_get_statements_with_pagination( assert third_response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_with_pagination_and_query( +def test_api_statements_get_with_pagination_and_query( monkeypatch, insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given a request with a query parameter @@ -639,7 +641,7 @@ def test_api_statements_get_statements_with_pagination_and_query( assert second_response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_with_no_matching_statement( +def test_api_statements_get_with_no_matching_statement( insert_statements_and_monkeypatch_backend, auth_credentials ): """Test the get statements API route, given a query yielding no matching statement, @@ -668,9 +670,7 @@ def test_api_statements_get_statements_with_no_matching_statement( assert response.json() == {"statements": []} -def test_api_statements_get_statements_with_database_query_failure( - auth_credentials, monkeypatch -): +def test_api_statements_get_with_database_query_failure(auth_credentials, monkeypatch): """Test the get statements API route, given a query raising a BackendException, should return an error response with HTTP code 500. """ @@ -694,9 +694,7 @@ def mock_query_statements(*_): @pytest.mark.parametrize("id_param", ["statementId", "voidedStatementId"]) -def test_api_statements_get_statements_invalid_query_parameters( - auth_credentials, id_param -): +def test_api_statements_get_invalid_query_parameters(auth_credentials, id_param): """Test error response for invalid query parameters""" id_1 = "be67b160-d958-4f51-b8b8-1892002dbac6" @@ -721,8 +719,8 @@ def test_api_statements_get_statements_invalid_query_parameters( # Check for 400 status code when invalid parameters are provided with a statementId for invalid_param, value in [ - ("activity", create_mock_activity()["id"]), - ("agent", json.dumps(create_mock_agent("mbox", 1))), + ("activity", mock_activity()["id"]), + ("agent", json.dumps(mock_agent("mbox", 1))), ("verb", "verb_1"), ]: response = client.get( diff --git a/tests/api/test_statements_post.py b/tests/api/test_statements_post.py index 5d9377979..350e4a11c 100644 --- a/tests/api/test_statements_post.py +++ b/tests/api/test_statements_post.py @@ -28,6 +28,7 @@ from ..helpers import ( assert_statement_get_responses_are_equivalent, + mock_statement, string_is_date, string_is_uuid, ) @@ -38,19 +39,7 @@ def test_api_statements_post_invalid_parameters(auth_credentials): """Test that using invalid parameters returns the proper status code.""" - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Check for 400 status code when unknown parameters are provided response = client.post( @@ -76,19 +65,7 @@ def test_api_statements_post_single_statement_directly( # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", @@ -184,17 +161,8 @@ def test_api_statements_post_enriching_with_existing_values( monkeypatch.setattr( "ralph.api.routers.statements.BACKEND_CLIENT", get_es_test_backend() ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "object": {"id": "https://example.com/object-id/1/"}, - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() + # Add the field to be tested statement[field] = value @@ -236,19 +204,7 @@ def test_api_statements_post_single_statement_no_trailing_slash( # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements", @@ -265,26 +221,14 @@ def test_api_statements_post_single_statement_no_trailing_slash( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_of_one( +def test_api_statements_post_list_of_one( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with one statement in a list.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", @@ -310,41 +254,21 @@ def test_api_statements_post_statements_list_of_one( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list( +def test_api_statements_post_list( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with two statements in a list.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statements = [ - { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:52Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - }, - { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - # Note the second statement has no preexisting ID - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - }, - ] + + statement_1 = mock_statement(timestamp="2022-03-15T14:07:52Z") + + # Note the second statement has no preexisting ID + statement_2 = mock_statement(timestamp="2022-03-15T14:07:51Z") + statement_2.pop("id") + + statements = [statement_1, statement_2] response = client.post( "/xAPI/statements/", @@ -381,26 +305,14 @@ def test_api_statements_post_statements_list( ], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_with_duplicates( +def test_api_statements_post_list_with_duplicates( backend, monkeypatch, auth_credentials, es_data_stream, mongo, clickhouse ): """Test the post statements API route with duplicate statement IDs should fail.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", @@ -426,7 +338,7 @@ def test_api_statements_post_statements_list_with_duplicates( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_with_duplicate_of_existing_statement( +def test_api_statements_post_list_with_duplicate_of_existing_statement( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the post statements API route, given a statement that already exist in the @@ -437,19 +349,7 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) statement_uuid = str(uuid4()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": statement_uuid, - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement(id_=statement_uuid) # Post the statement once. response = client.post( @@ -499,7 +399,7 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen "backend", [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) -def test_api_statements_post_statements_with_a_failure_during_storage( +def test_api_statements_post_with_failure_during_storage( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with a failure happening during storage.""" @@ -512,19 +412,7 @@ def write_mock(*args, **kwargs): backend_instance = backend() monkeypatch.setattr(backend_instance, "write", write_mock) monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend_instance) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", @@ -540,7 +428,7 @@ def write_mock(*args, **kwargs): "backend", [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) -def test_api_statements_post_statements_with_a_failure_during_id_query( +def test_api_statements_post_with_failure_during_id_query( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with a failure during query execution.""" @@ -555,19 +443,7 @@ def query_statements_by_ids_mock(*args, **kwargs): backend_instance, "query_statements_by_ids", query_statements_by_ids_mock ) monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend_instance) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", @@ -584,7 +460,7 @@ def query_statements_by_ids_mock(*args, **kwargs): [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_without_statement_forwarding( +def test_api_statements_post_list_without_forwarding( backend, auth_credentials, monkeypatch, es, mongo, clickhouse ): """Test the post statements API route, given an empty forwarding configuration, @@ -607,19 +483,7 @@ def spy_mock_forward_xapi_statements(_): ) monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", @@ -652,7 +516,7 @@ def spy_mock_forward_xapi_statements(_): ), ], ) -async def test_api_statements_post_statements_list_with_statement_forwarding( +async def test_api_statements_post_list_with_forwarding( receiving_backend, forwarding_backend, monkeypatch, @@ -670,19 +534,7 @@ async def test_api_statements_post_statements_list_with_statement_forwarding( """ # pylint: disable=invalid-name,unused-argument,too-many-arguments,too-many-locals - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Set-up receiving LRS client with monkeypatch.context() as receiving_patch: diff --git a/tests/api/test_statements_put.py b/tests/api/test_statements_put.py index a4f43e29c..330bccd0f 100644 --- a/tests/api/test_statements_put.py +++ b/tests/api/test_statements_put.py @@ -25,26 +25,18 @@ get_mongo_test_backend, ) -from ..helpers import assert_statement_get_responses_are_equivalent, string_is_date +from ..helpers import ( + assert_statement_get_responses_are_equivalent, + mock_statement, + string_is_date, +) client = TestClient(app) def test_api_statements_put_invalid_parameters(auth_credentials): """Test that using invalid parameters returns the proper status code.""" - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Check for 400 status code when unknown parameters are provided response = client.put( @@ -70,19 +62,7 @@ def test_api_statements_put_single_statement_directly( # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", @@ -113,18 +93,7 @@ def test_api_statements_put_enriching_without_existing_values( monkeypatch.setattr( "ralph.api.routers.statements.BACKEND_CLIENT", get_es_test_backend() ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "object": {"id": "https://example.com/object-id/1/"}, - "verb": {"id": "https://example.com/verb-id/1/"}, - "id": str(uuid4()), - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", @@ -176,18 +145,8 @@ def test_api_statements_put_enriching_with_existing_values( monkeypatch.setattr( "ralph.api.routers.statements.BACKEND_CLIENT", get_es_test_backend() ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "object": {"id": "https://example.com/object-id/1/"}, - "verb": {"id": "https://example.com/verb-id/1/"}, - "id": str(uuid4()), - } + statement = mock_statement() + # Add the field to be tested statement[field] = value @@ -228,19 +187,7 @@ def test_api_statements_put_single_statement_no_trailing_slash( # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements?statementId={statement['id']}", @@ -256,25 +203,13 @@ def test_api_statements_put_single_statement_no_trailing_slash( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_put_statement_id_mismatch( +def test_api_statements_put_id_mismatch( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): # pylint: disable=invalid-name,unused-argument """Test the put statements API route when the statementId doesn't match.""" monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement(id_=str(uuid4())) different_statement_id = str(uuid4()) response = client.put( @@ -294,25 +229,13 @@ def test_api_statements_put_statement_id_mismatch( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_put_statements_list_of_one( +def test_api_statements_put_list_of_one( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): # pylint: disable=invalid-name,unused-argument """Test that we fail on PUTs with a list, even if it's one statement.""" monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", @@ -328,7 +251,7 @@ def test_api_statements_put_statements_list_of_one( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_put_statement_duplicate_of_existing_statement( +def test_api_statements_put_duplicate_of_existing_statement( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the put statements API route, given a statement that already exist in the @@ -337,19 +260,7 @@ def test_api_statements_put_statement_duplicate_of_existing_statement( # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Put the statement once. response = client.put( @@ -387,7 +298,7 @@ def test_api_statements_put_statement_duplicate_of_existing_statement( "backend", [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) -def test_api_statements_put_statements_with_a_failure_during_storage( +def test_api_statements_put_with_failure_during_storage( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the put statements API route with a failure happening during storage.""" @@ -400,19 +311,7 @@ def write_mock(*args, **kwargs): backend_instance = backend() monkeypatch.setattr(backend_instance, "write", write_mock) monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend_instance) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", @@ -428,7 +327,7 @@ def write_mock(*args, **kwargs): "backend", [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) -def test_api_statements_put_statement_with_a_failure_during_id_query( +def test_api_statements_put_with_a_failure_during_id_query( backend, monkeypatch, auth_credentials, es, mongo, clickhouse ): """Test the put statements API route with a failure during query execution.""" @@ -443,19 +342,7 @@ def query_statements_by_ids_mock(*args, **kwargs): backend_instance, "query_statements_by_ids", query_statements_by_ids_mock ) monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend_instance) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", @@ -472,7 +359,7 @@ def query_statements_by_ids_mock(*args, **kwargs): [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_put_statement_without_statement_forwarding( +def test_api_statements_put_without_forwarding( backend, auth_credentials, monkeypatch, es, mongo, clickhouse ): """Test the put statements API route, given an empty forwarding configuration, @@ -495,19 +382,7 @@ def spy_mock_forward_xapi_statements(_): ) monkeypatch.setattr("ralph.api.routers.statements.BACKEND_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", @@ -539,7 +414,7 @@ def spy_mock_forward_xapi_statements(_): ), ], ) -async def test_api_statements_put_statement_with_statement_forwarding( +async def test_api_statements_put_with_forwarding( receiving_backend, forwarding_backend, monkeypatch, @@ -557,19 +432,7 @@ async def test_api_statements_put_statement_with_statement_forwarding( """ # pylint: disable=invalid-name,unused-argument,too-many-arguments,too-many-locals - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Set-up receiving LRS client with monkeypatch.context() as receiving_patch: diff --git a/tests/backends/http/test_async_lrs.py b/tests/backends/http/test_async_lrs.py index 44d4a1ff0..f89f21aec 100644 --- a/tests/backends/http/test_async_lrs.py +++ b/tests/backends/http/test_async_lrs.py @@ -3,12 +3,9 @@ import asyncio import json import logging -import random import time -from datetime import datetime from functools import partial from urllib.parse import ParseResult, parse_qsl, urlencode, urljoin, urlparse -from uuid import uuid4 import httpx import pytest @@ -26,6 +23,8 @@ from ralph.backends.http.base import HTTPBackendStatus from ralph.exceptions import BackendException, BackendParameterException +from ...helpers import mock_statement + # pylint: disable=too-many-lines @@ -37,26 +36,6 @@ async def _unpack_async_generator(async_gen): return result -def _gen_statement(id_=None, verb=None, timestamp=None): - """Generate fake statements with random or provided parameters.""" - if id_ is None: - id_ = str(uuid4()) - if verb is None: - verb = {"id": f"https://w3id.org/xapi/video/verbs/{random.random()}"} - elif isinstance(verb, int): - verb = {"id": f"https://w3id.org/xapi/video/verbs/{verb}"} - if timestamp is None: - timestamp = datetime.strftime( - datetime.fromtimestamp(time.time() - random.random()), - "%Y-%m-%dT%H:%M:%S", - ) - elif isinstance(timestamp, int): - timestamp = datetime.strftime( - datetime.fromtimestamp((time.time() - timestamp), "%Y-%m-%dT%H:%M:%S") - ) - return {"id": id_, "verb": verb, "timestamp": timestamp} - - def test_backend_http_lrs_default_instantiation( monkeypatch, fs ): # pylint:disable = invalid-name @@ -250,11 +229,11 @@ async def test_backends_http_lrs_read_max_statements( chunk_size = 3 statements = { - "statements": [_gen_statement() for _ in range(chunk_size)], + "statements": [mock_statement() for _ in range(chunk_size)], "more": more_target, } more_statements = { - "statements": [_gen_statement() for _ in range(chunk_size)], + "statements": [mock_statement() for _ in range(chunk_size)], } # Mock GET response of HTTPX for target and "more" target without query parameter @@ -328,7 +307,7 @@ async def test_backends_http_lrs_read_without_target( ) backend = AsyncLRSHTTPBackend(settings) - statements = {"statements": [_gen_statement() for _ in range(3)]} + statements = {"statements": [mock_statement() for _ in range(3)]} # Mock HTTPX GET default_params = LRSStatementsQuery(limit=500).dict( @@ -419,9 +398,9 @@ async def test_backends_http_lrs_read_without_pagination( statements = { "statements": [ - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), ] } @@ -517,19 +496,19 @@ async def test_backends_http_lrs_read_with_pagination(httpx_mock: HTTPXMock): more_target = "/xAPI/statements/?pit_id=fake-pit-id" statements = { "statements": [ - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement( + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement( verb={"id": "https://w3id.org/xapi/video/verbs/initialized"} ), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), ], "more": more_target, } more_statements = { "statements": [ - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/seeked"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/seeked"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), ] } @@ -672,7 +651,7 @@ async def test_backends_http_lrs_write_without_operation( base_url = "http://fake-lrs.com" target = "/xAPI/statements/" - data = [_gen_statement() for _ in range(6)] + data = [mock_statement() for _ in range(6)] settings = LRSHTTPBackendSettings( BASE_URL=base_url, @@ -831,7 +810,7 @@ async def test_backends_http_lrs_write_without_target(httpx_mock: HTTPXMock, cap ) backend = AsyncLRSHTTPBackend(settings) - data = [_gen_statement() for _ in range(3)] + data = [mock_statement() for _ in range(3)] # Mock HTTPX POST httpx_mock.add_response( @@ -871,7 +850,7 @@ async def test_backends_http_lrs_write_with_create_or_index_operation( ) backend = AsyncLRSHTTPBackend(settings) - data = [_gen_statement() for _ in range(3)] + data = [mock_statement() for _ in range(3)] # Mock HTTPX POST httpx_mock.add_response(url=urljoin(base_url, target), method="POST", json=data) @@ -905,7 +884,7 @@ async def test_backends_http_lrs_write_backend_exception( ) backend = AsyncLRSHTTPBackend(settings) - data = [_gen_statement()] + data = [mock_statement()] # Mock HTTPX POST httpx_mock.add_response( @@ -968,7 +947,7 @@ async def _simulate_slow_processing(): all_statements = {} for index in range(num_pages): all_statements[index] = { - "statements": [_gen_statement() for _ in range(chunk_size)] + "statements": [mock_statement() for _ in range(chunk_size)] } if index < num_pages - 1: all_statements[index]["more"] = targets[index + 1] @@ -1032,7 +1011,7 @@ async def test_backends_http_lrs_write_concurrency( base_url = "http://fake-lrs.com" - data = [_gen_statement() for _ in range(6)] + data = [mock_statement() for _ in range(6)] # Changing data length might break tests assert len(data) == 6 diff --git a/tests/fixtures/auth.py b/tests/fixtures/auth.py index 23173ed85..da4c83868 100644 --- a/tests/fixtures/auth.py +++ b/tests/fixtures/auth.py @@ -22,7 +22,7 @@ PUBLIC_KEY_ID = "example-key-id" -def create_user( +def mock_basic_auth_user( fs_, username: str, password: str, @@ -91,7 +91,7 @@ def auth_credentials(fs, user_scopes=None, agent=None): if agent is None: agent = {"mbox": "mailto:test_ralph@example.com"} - credentials = create_user(fs, username, password, user_scopes, agent) + credentials = mock_basic_auth_user(fs, username, password, user_scopes, agent) return credentials diff --git a/tests/helpers.py b/tests/helpers.py index 6d3fdb223..3aceccad1 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,8 +1,11 @@ """Utilities for testing Ralph.""" -import datetime import hashlib +import random +import time import uuid -from typing import Optional +from datetime import datetime +from typing import Optional, Union +from uuid import UUID from ralph.utils import statements_are_equivalent @@ -10,7 +13,7 @@ def string_is_date(string: str): """Check if string can be parsed as a date.""" try: - datetime.datetime.fromisoformat(string) + datetime.fromisoformat(string) return True except ValueError: return False @@ -52,7 +55,7 @@ def _all_but_statements(response): ), "Statements in get responses are not equivalent, or not in the same order." -def create_mock_activity(id_: int = 0): +def mock_activity(id_: int = 0): """Create distinct activites with valid IRIs. Args: @@ -65,9 +68,9 @@ def create_mock_activity(id_: int = 0): } -def create_mock_agent( - ifi: str, - id_: int, +def mock_agent( + ifi: str = "mbox", + id_: int = 1, home_page_id: Optional[int] = None, name: Optional[str] = None, use_object_type: bool = True, @@ -111,7 +114,7 @@ def create_mock_agent( if ifi == "account": if home_page_id is None: raise ValueError( - "home_page_id must be defined if using create_mock_agent if " + "home_page_id must be defined if using mock_agent if " "using ifi=='account'" ) agent["account"] = { @@ -120,4 +123,76 @@ def create_mock_agent( } return agent - raise ValueError("No valid ifi was provided to create_mock_agent") + raise ValueError("No valid ifi was provided to mock_agent") + + +def mock_statement( + id_: Optional[Union[UUID, int]] = None, + actor: Optional[Union[dict, int]] = None, + verb: Optional[Union[dict, int]] = None, + object: Optional[Union[dict, int]] = None, + timestamp: Optional[Union[str, int]] = None, +): + """Generate fake statements with random or provided parameters. + Fields `actor`, `verb`, `object`, `timestamp` accept integer values which + can be used to create distinct values identifiable by this integer. For each + variable, using `None` will assign a default value. `timestamp` may be ommited + by using value `""` + Args: + id_: id of the statement + actor: actor of the statement + verb: verb of the statement + object: object of the statement + timestamp: timestamp of the statement. Use `""` to omit timestamp + """ + # pylint: disable=redefined-builtin + + # Id + if id_ is None: + id_ = str(uuid.uuid4()) + + # Actor + if actor is None: + actor = mock_agent() + elif isinstance(actor, int): + actor = mock_agent(id_=actor) + + # Verb + if verb is None: + verb = {"id": f"https://w3id.org/xapi/video/verbs/{random.random()}"} + elif isinstance(verb, int): + verb = {"id": f"https://w3id.org/xapi/video/verbs/{verb}"} + + # Object + if object is None: + object = { + "id": f"http://example.adlnet.gov/xapi/example/activity_{random.random()}" + } + elif isinstance(object, int): + object = {"id": f"http://example.adlnet.gov/xapi/example/activity_{object}"} + + # Timestamp + if timestamp is None: + timestamp = datetime.strftime( + datetime.fromtimestamp(time.time() - random.random()), + "%Y-%m-%dT%H:%M:%S+00:00", + ) + elif isinstance(timestamp, int): + timestamp = datetime.strftime( + datetime.fromtimestamp(1696236665 + timestamp), "%Y-%m-%dT%H:%M:%S+00:00" + ) + elif timestamp == "": + return { + "id": id_, + "actor": actor, + "verb": verb, + "object": object, + } + + return { + "id": id_, + "actor": actor, + "verb": verb, + "object": object, + "timestamp": timestamp, + } diff --git a/tests/test_helpers.py b/tests/test_helpers.py index fc5177665..e40b8dff9 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -6,6 +6,7 @@ from .helpers import ( assert_statement_get_responses_are_equivalent, + mock_statement, string_is_date, string_is_uuid, ) @@ -121,3 +122,63 @@ def test_helpers_assert_statement_get_responses_are_equivalent_length_error(): assert_statement_get_responses_are_equivalent(get_response_1, get_response_2) with pytest.raises(AssertionError): assert_statement_get_responses_are_equivalent(get_response_2, get_response_1) + + +def test_helpers_mock_statement_no_input(): + """Test that mocked statement have the expected fields.""" + + statement = mock_statement() + + assert "id" in statement + assert "actor" in statement + assert "verb" in statement + assert "object" in statement + assert "timestamp" in statement + + statement = mock_statement(timestamp="") + assert "timestamp" not in statement + + +def test_helpers_mock_statement_value_input(): + """Test that mocked statement have the expected fields with value input.""" + + reference_statement = { + "id": str(uuid4()), + "actor": { + "account": { + "homePage": "https://example.com/homepage/", + "name": str(uuid4()), + }, + "objectType": "Agent", + }, + # Note the second statement has no preexisting ID + "object": {"id": "https://example.com/object-id/1/"}, + "timestamp": "2022-03-15T14:07:51Z", + "verb": {"id": "https://example.com/verb-id/1/"}, + } + + statement = mock_statement( + id_=reference_statement["id"], + actor=reference_statement["actor"], + verb=reference_statement["verb"], + object=reference_statement["object"], + timestamp=reference_statement["timestamp"], + ) + + assert statement == reference_statement + + +@pytest.mark.parametrize("field", ["actor", "verb", "object", "timestamp"]) +@pytest.mark.parametrize("integer", [0, 1, 5]) +def test_helpers_mock_statement_integer_input(field, integer): + """Test that mocked statement fields behave properly with integer input.""" + + # Test that fields have same values for same integer input + statement_1 = mock_statement(**{field: integer}) + statement_2 = mock_statement(**{field: integer}) + assert statement_1[field] == statement_2[field] + + # Test that fields have different values for different integer input + statement_1 = mock_statement(**{field: integer}) + statement_2 = mock_statement(**{field: integer + 1}) + assert statement_1[field] != statement_2[field]