Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DISCO-3182 Remove duplicated fixtures #773

Merged
merged 1 commit into from
Jan 29, 2025
Merged

DISCO-3182 Remove duplicated fixtures #773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion merino/providers/suggest/top_picks/backends/filemanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from typing import Any

from merino.exceptions import FilemanagerError
from merino.utils.gcs.gcp_uploader import GcsUploader
from merino.utils.gcs.gcs_uploader import GcsUploader

logger = logging.getLogger(__name__)

Expand Down
16 changes: 4 additions & 12 deletions tests/unit/providers/manifest/backends/test_filemanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@

def test_get_file(
manifest_remote_filemanager: ManifestRemoteFilemanager,
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
blob_json,
) -> None:
Expand All @@ -25,6 +23,7 @@ def test_get_file(

get_file_result_code, result = manifest_remote_filemanager.get_file()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we add a success check for get_file_result_code or change to _ since it's an unused variable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!


assert get_file_result_code is GetManifestResultCode.SUCCESS
assert isinstance(result, ManifestData)
assert result.domains
assert len(result.domains) == 3
Expand All @@ -33,8 +32,6 @@ def test_get_file(

def test_get_file_skip(
manifest_remote_filemanager: ManifestRemoteFilemanager,
gcs_client_mock,
gcs_bucket_mock,
) -> None:
"""Test that the get_file method returns the SKIP code when there's no new generation."""
manifest_remote_filemanager.gcs_client = MagicMock()
Expand All @@ -49,7 +46,6 @@ def test_get_file_skip(
def test_get_file_fail(
manifest_remote_filemanager: ManifestRemoteFilemanager,
gcs_client_mock,
gcs_bucket_mock,
) -> None:
"""Test that the get_file method returns the FAIL code on failure."""
gcs_client_mock.get_bucket.side_effect = Exception("Test error")
Expand All @@ -65,15 +61,12 @@ def test_get_file_fail_validation_error(
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
blob_json,
) -> None:
"""Test that the get_file method returns the FAIL code when a validation error occurs."""
mock_blob = gcs_blob_mock(blob_json, "manifest.json")

gcs_bucket_mock.get_blob.return_value = mock_blob
gcs_bucket_mock.get_blob.return_value = gcs_blob_mock
gcs_client_mock.get_bucket.return_value = gcs_bucket_mock

mock_blob.download_as_text.return_value = '{"invalid": "data"}'
gcs_blob_mock.download_as_text.return_value = '{"invalid": "data"}'

with patch(
"merino.providers.manifest.backends.filemanager.ManifestData.model_validate",
Expand All @@ -90,10 +83,9 @@ def test_get_file_fail_json_decoder_error(
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
blob_json,
) -> None:
"""Test that the get_file method returns the FAIL code when a JSON decoder occurs."""
gcs_bucket_mock.get_blob.return_value = gcs_blob_mock(blob_json, "manifest.json")
gcs_bucket_mock.get_blob.return_value = gcs_blob_mock
gcs_client_mock.get_bucket.return_value = gcs_bucket_mock

with patch(
Expand Down
11 changes: 0 additions & 11 deletions tests/unit/providers/manifest/backends/test_manifest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@
def test_fetch_manifest_data(
manifest_remote_filemanager: ManifestRemoteFilemanager,
backend: ManifestBackend,
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
blob_json,
) -> None:
"""Verify that the fetch_manifest_data method returns manifest data on success"""
manifest_remote_filemanager.gcs_client = MagicMock()
Expand All @@ -38,9 +35,6 @@ def test_fetch_manifest_data(

def test_fetch_manifest_data_skip(
manifest_remote_filemanager: ManifestRemoteFilemanager,
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
backend: ManifestBackend,
) -> None:
"""Verify that the fetch_manifest_data method returns SKIP code for no new blob generation"""
Expand All @@ -59,8 +53,6 @@ def test_fetch_manifest_data_skip(

def test_fetch_manifest_data_fail(
manifest_remote_filemanager: ManifestRemoteFilemanager,
gcs_client_mock,
gcs_bucket_mock,
backend: ManifestBackend,
) -> None:
"""Verify that the fetch_manifest_data method returns FAIL code when an error occurs"""
Expand Down Expand Up @@ -88,10 +80,7 @@ def test_fetch_manifest_data_fail(
async def test_fetch(
manifest_remote_filemanager: ManifestRemoteFilemanager,
backend: ManifestBackend,
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
blob_json,
) -> None:
"""Verify that the fetch method returns manifest data"""
manifest_remote_filemanager.gcs_client = MagicMock()
Expand Down
6 changes: 1 addition & 5 deletions tests/unit/providers/suggest/top_picks/backends/test_filemanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def fixture_top_picks_remote_filemanager(
top_picks_remote_filemanager_parameters: dict[str, Any], gcs_client_mock
) -> TopPicksRemoteFilemanager:
"""Create a TopPicksRemoteFilemanager object for test."""
with patch("merino.utils.gcs.gcp_uploader.Client") as mock_client, patch(
with patch("merino.utils.gcs.gcs_uploader.Client") as mock_client, patch(
"google.auth.default"
) as mock_auth_default:
creds = AnonymousCredentials() # type: ignore
Expand Down Expand Up @@ -116,15 +116,11 @@ def test_get_file(
top_picks_remote_filemanager: TopPicksRemoteFilemanager,
caplog: LogCaptureFixture,
filter_caplog: FilterCaplogFixture,
gcs_client_mock,
gcs_bucket_mock,
gcs_blob_mock,
blob_json,
) -> None:
"""Test that the Remote Filemanager get_file method returns domain data."""
top_picks_remote_filemanager.gcs_client = MagicMock()
top_picks_remote_filemanager.gcs_client.get_file_by_name.return_value = gcs_blob_mock
gcs_blob_mock.download_as_text.return_value = blob_json

caplog.set_level(logging.INFO)
get_file_result_code, result = top_picks_remote_filemanager.get_file()
Expand Down
36 changes: 1 addition & 35 deletions tests/unit/providers/suggest/top_picks/backends/test_top_picks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
from unittest.mock import MagicMock

import pytest
from google.cloud.storage import Blob, Bucket, Client
from pytest import LogCaptureFixture
from pytest_mock import MockerFixture

from merino.configs import settings
from merino.providers.suggest.top_picks.backends.filemanager import GetFileResultCode
Expand Down Expand Up @@ -46,38 +44,6 @@ def fixture_top_picks(top_picks_backend_parameters: dict[str, Any]) -> TopPicksB
return TopPicksBackend(**top_picks_backend_parameters)


@pytest.fixture(name="expected_timestamp")
def fixture_expected_timestamp() -> int:
"""Return a unix timestamp for metadata mocking."""
return 16818664520924621


@pytest.fixture(name="gcs_blob_mock", autouse=True)
def fixture_gcs_blob_mock(mocker: MockerFixture, expected_timestamp: int, blob_json: str) -> Any:
"""Create a GCS Blob mock object for testing."""
mock_blob = mocker.MagicMock(spec=Blob)
mock_blob.name = "20220101120555_top_picks.json"
mock_blob.generation = expected_timestamp
mock_blob.download_as_text.return_value = blob_json
return mock_blob


@pytest.fixture(name="gcs_bucket_mock", autouse=True)
def fixture_gcs_bucket_mock(mocker: MockerFixture, gcs_blob_mock) -> Any:
"""Create a GCS Bucket mock object for testing."""
mock_bucket = mocker.MagicMock(spec=Bucket)
mock_bucket.get_blob.return_value = gcs_blob_mock
return mock_bucket


@pytest.fixture(name="gcs_client_mock", autouse=True)
def mock_gcs_client(mocker: MockerFixture, gcs_bucket_mock):
"""Return a mock GCS Client instance"""
mock_client = mocker.MagicMock(spec=Client)
mock_client.get_bucket.return_value = gcs_bucket_mock
return mock_client


def test_init_failure_no_domain_file(
top_picks_backend_parameters: dict[str, Any],
) -> None:
Expand Down Expand Up @@ -162,10 +128,10 @@ def test_maybe_build_indicies_local(
def test_maybe_build_indicies_remote(
top_picks_backend: TopPicksBackend,
top_picks_remote_filemanager,
gcs_blob_mock,
mocker,
caplog: LogCaptureFixture,
filter_caplog: FilterCaplogFixture,
gcs_blob_mock,
) -> None:
"""Test remote build indicies method."""
caplog.set_level(logging.INFO)
Expand Down
68 changes: 0 additions & 68 deletions tests/unit/providers/suggest/top_picks/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

"""Module for test configurations for the Top Picks provider unit test directory."""

import json
from typing import Any

import pytest
Expand Down Expand Up @@ -81,70 +80,3 @@ def fixture_top_picks_parameters() -> dict[str, Any]:
def fixture_top_picks(backend: TopPicksBackend, top_picks_parameters: dict[str, Any]) -> Provider:
"""Create Top Pick Provider for test."""
return Provider(backend=backend, **top_picks_parameters) # type: ignore [arg-type]


@pytest.fixture(name="blob_json")
def fixture_blob_json() -> str:
"""Return a JSON string for mocking."""
return json.dumps(
{
"domains": [
{
"rank": 1,
"title": "Example",
"domain": "example",
"url": "https://example.com",
"icon": "",
"categories": ["web-browser"],
"serp_categories": [0],
"similars": ["exxample", "exampple", "eexample"],
},
{
"rank": 2,
"title": "Firefox",
"domain": "firefox",
"url": "https://firefox.com",
"icon": "",
"categories": ["web-browser"],
"serp_categories": [0],
"similars": [
"firefoxx",
"foyerfox",
"fiirefox",
"firesfox",
"firefoxes",
],
},
{
"rank": 3,
"title": "Mozilla",
"domain": "mozilla",
"url": "https://mozilla.org/en-US/",
"icon": "",
"categories": ["web-browser"],
"serp_categories": [0],
"similars": ["mozzilla", "mozila"],
},
{
"rank": 4,
"title": "Abc",
"domain": "abc",
"url": "https://abc.test",
"icon": "",
"categories": ["web-browser"],
"serp_categories": [0],
"similars": ["aa", "ab", "acb", "acbc", "aecbc"],
},
{
"rank": 5,
"title": "BadDomain",
"domain": "baddomain",
"url": "https://baddomain.test",
"icon": "",
"categories": ["web-browser"],
"serp_categories": [0],
"similars": ["bad", "badd"],
},
]
}
)