Skip to content

Commit

Permalink
Merge pull request #186 from nelc/shadinaif/enhance-test-quality
Browse files Browse the repository at this point in the history 
refactor: enhance a few tests to use base_data fixture
  • Loading branch information
@shadinaif
shadinaif authored Jan 26, 2025
2 parents 8cb5115 + a7ca95e commit 8832ee1
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 85 deletions.
102 changes: 56 additions & 46 deletions tests/test_helpers/test_export_csv.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from django.contrib.auth import get_user_model
from django.core.files.storage import default_storage
from django.test import override_settings
from eox_tenant.models import TenantConfig
from storages.backends.s3boto3 import S3Boto3Storage

from futurex_openedx_extensions.helpers.exceptions import FXCodedException
Expand All @@ -32,22 +31,13 @@


@pytest.fixture
def user(db): # pylint: disable=unused-argument
return get_user_model().objects.create_user(username='testuser', password='password')


@pytest.fixture
def tenant(db): # pylint: disable=unused-argument
return TenantConfig.objects.create(external_key='test')


@pytest.fixture
def fx_task(db, user, tenant): # pylint: disable=unused-argument,redefined-outer-name
def fx_task():
"""Fixture for DataExportTask."""
return DataExportTask.objects.create(
filename=_FILENAME,
view_name='fake',
user=user,
tenant_id=tenant.id
user_id=30,
tenant_id=1,
)


Expand All @@ -67,9 +57,10 @@ def test_export_data_to_csv_invalid_user(
assert str(exc_info.value) == f'CSV Export: Invalid user id: {user_id}'


def test_get_user_valid_id(user): # pylint: disable=redefined-outer-name
@pytest.mark.django_db
def test_get_user_valid_id():
"""Test _get_user with a valid user_id."""
assert _get_user(user.id) == user
assert _get_user(30) == get_user_model().objects.get(id=30)


def test_get_view_class_instance():
Expand All @@ -91,9 +82,11 @@ def test_export_data_to_csv_invalid_path(path, base_data): # pylint: disable=un
assert str(exc_info.value) == f'CSV Export: Missing required params "path" {path}'


@pytest.mark.django_db
@override_settings(ALLOWED_HOSTS=['test-url.somthing'])
def test_get_mocked_request(user): # pylint: disable=redefined-outer-name
def test_get_mocked_request(base_data): # pylint: disable=unused-argument
"""Test _get_mocked_request creates a mocked request properly."""
user = get_user_model().objects.get(id=30)
fx_info = {'role': 'admin', 'user': user}
url = 'http://test-url.somthing/?test=123'
request = _get_mocked_request(url, fx_info)
Expand All @@ -102,9 +95,10 @@ def test_get_mocked_request(user): # pylint: disable=redefined-outer-name
assert request.fx_permission_info == fx_info


def test_get_mocked_request_invalid_url(user): # pylint: disable=redefined-outer-name
@pytest.mark.django_db
def test_get_mocked_request_invalid_url(base_data): # pylint: disable=unused-argument
"""Test _get_mocked_request creates a mocked request properly."""
fx_info = {'role': 'admin', 'user': user}
fx_info = {'role': 'admin', 'user': get_user_model().objects.get(id=30)}
url = '/test-url/?test=123'
with pytest.raises(FXCodedException) as exc_info:
_get_mocked_request(url, fx_info)
Expand Down Expand Up @@ -143,12 +137,13 @@ def test_get_response_data_failure(
assert str(exc_info.value) == exception_msg


@pytest.mark.django_db
@override_settings(ALLOWED_HOSTS=['example.com'])
@patch('futurex_openedx_extensions.helpers.export_csv._get_response_data')
def test_paginated_response_generator(mock_get_response_data):
"""Test _paginated_response_generator"""
url = 'http://example.com/api/data'
fx_info = {'role': 'admin', 'user': user}
fx_info = {'role': 'admin', 'user': get_user_model().objects.get(id=30)}
page_size = 2
view_data = {'url': f'{url}?test=value&page_size={page_size}'}
mocked_response_1 = MagicMock()
Expand Down Expand Up @@ -184,11 +179,14 @@ def test_paginated_response_generator(mock_get_response_data):
assert view_instance.call_count == 2


@pytest.mark.django_db
@override_settings(ALLOWED_HOSTS=['example.com'])
@patch('futurex_openedx_extensions.helpers.export_csv._get_response_data')
def test_paginated_response_generator_for_empty_response_data(mock_get_response_data):
def test_paginated_response_generator_for_empty_response_data(
mock_get_response_data, base_data,
): # pylint: disable=unused-argument
"""Test _paginated_response_generator for empty response when there are no records"""
fx_info = {'role': 'admin', 'user': user}
fx_info = {'role': 'admin', 'user': get_user_model().objects.get(id=30)}
view_data = {'url': 'http://example.com/api/data?test=value&page_size=2'}
mocked_response = MagicMock()
mocked_response.status_code = 200
Expand All @@ -205,10 +203,11 @@ def test_paginated_response_generator_for_empty_response_data(mock_get_response_
view_instance.assert_called_once()


def test_get_storage_dir(tenant): # pylint: disable=redefined-outer-name
def test_get_storage_dir():
"""Return storgae dir"""
expected = os.path.join(settings.FX_DASHBOARD_STORAGE_DIR, f'{str(tenant.id)}/exported_files')
result = _get_storage_dir(str(tenant.id))
tenant_id = 1
expected = os.path.join(settings.FX_DASHBOARD_STORAGE_DIR, f'{str(tenant_id)}/exported_files')
result = _get_storage_dir(str(tenant_id))
assert result == expected


Expand Down Expand Up @@ -262,17 +261,18 @@ def test_upload_file_to_storage_set_private(mock_get_storage_dir, mock_storage):
@patch('futurex_openedx_extensions.helpers.export_csv._paginated_response_generator')
@patch('futurex_openedx_extensions.helpers.models.DataExportTask.get_task')
def test_generate_csv_with_tracked_progress(
mock_get_task, mock_generator, tenant, base_data,
): # pylint: disable=redefined-outer-name, unused-argument
mock_get_task, mock_generator, base_data,
): # pylint: disable=unused-argument
"""Test _generate_csv_with_tracked_progress."""
tenant = MagicMock(id=1)
task = MagicMock()
task.tenant = tenant
task.status = DataExportTask.STATUS_PROCESSING
mock_get_task.return_value = task

storage_dir = f'{settings.FX_DASHBOARD_STORAGE_DIR}/{str(tenant.id)}/exported_files'
fake_storage_path = f'{storage_dir}/{_FILENAME}'
fx_permission_info = {'user': user, 'role': 'admin'}
fx_permission_info = {'user': get_user_model().objects.get(id=30), 'role': 'admin'}
view_data = {
'page_size': 2,
'url': 'http://example.com',
Expand Down Expand Up @@ -312,7 +312,7 @@ def test_generate_csv_with_tracked_progress_for_exception(
): # pylint: disable=unused-argument
"""Test _generate_csv_with_tracked_progress for exception."""
task = MagicMock()
fx_permission_info = {'user': user, 'role': 'admin'}
fx_permission_info = {'user': get_user_model().objects.get(id=30), 'role': 'admin'}
view_data = {
'page_size': 2,
'url': 'http://example.com',
Expand All @@ -333,9 +333,10 @@ def test_generate_csv_with_tracked_progress_for_exception(
@patch('futurex_openedx_extensions.helpers.export_csv.os.remove')
@patch('futurex_openedx_extensions.helpers.models.DataExportTask.get_task')
def test_generate_csv_with_tracked_progress_for_os_removal_exception(
mock_get_task, mock_os_remove, mock_generator, mock_file_upload, tenant, base_data,
): # pylint: disable=redefined-outer-name, unused-argument, too-many-arguments
mock_get_task, mock_os_remove, mock_generator, mock_file_upload, base_data,
): # pylint: disable=unused-argument
"""Test _generate_csv_with_tracked_progress for os exception"""
tenant = MagicMock(id=1)
task = MagicMock()
task.id = 99
task.tenant_id = tenant.id
Expand Down Expand Up @@ -364,12 +365,13 @@ def test_generate_csv_with_tracked_progress_for_os_removal_exception(
@pytest.mark.django_db
@patch('futurex_openedx_extensions.helpers.export_csv._paginated_response_generator')
def test_generate_csv_with_tracked_progress_for_empty_records(
mock_generator, fx_task, tenant, base_data,
mock_generator, fx_task, base_data,
): # pylint: disable=redefined-outer-name, unused-argument
"""_generate_csv_with_tracked_progress for empty records"""
tenant = MagicMock(id=1)
storage_dir = f'{settings.FX_DASHBOARD_STORAGE_DIR}/{str(tenant.id)}/exported_files'
fake_storage_path = f'{storage_dir}/{_FILENAME}'
fx_permission_info = {'user': user, 'role': 'admin'}
fx_permission_info = {'user': get_user_model().objects.get(id=30), 'role': 'admin'}
view_data = {
'page_size': 2,
'url': 'http://example.com',
Expand All @@ -389,12 +391,14 @@ def test_generate_csv_with_tracked_progress_for_empty_records(
os.rmdir(settings.FX_DASHBOARD_STORAGE_DIR)


@pytest.mark.django_db
@patch('futurex_openedx_extensions.helpers.export_csv._get_view_class_instance')
@patch('futurex_openedx_extensions.helpers.export_csv._generate_csv_with_tracked_progress')
def test_export_data_to_csv(
mock_generate_csv, mock_get_view, fx_task, user
): # pylint: disable=redefined-outer-name
mock_generate_csv, mock_get_view, fx_task, base_data,
): # pylint: disable=redefined-outer-name, unused-argument
"""Test _export_data_to_csv"""
user = get_user_model().objects.get(id=30)
mock_view_instance = MagicMock()
mock_view_instance.view_class.pagination_class.max_page_size = 50
mock_get_view.return_value = mock_view_instance
Expand All @@ -419,12 +423,14 @@ def test_export_data_to_csv(
mock_get_view.assert_called_once_with(view_data['path'])


@pytest.mark.django_db
@patch('futurex_openedx_extensions.helpers.export_csv._get_view_class_instance')
@patch('futurex_openedx_extensions.helpers.export_csv._generate_csv_with_tracked_progress')
def test_export_data_to_csv_for_default_page_size(
mock_generate_csv, mock_get_view, fx_task, user
): # pylint: disable=redefined-outer-name
mock_generate_csv, mock_get_view, fx_task, base_data,
): # pylint: disable=redefined-outer-name, unused-argument
"""Test _export_data_to_csv"""
user = get_user_model().objects.get(id=30)
fake_storage_path = f'{settings.FX_DASHBOARD_STORAGE_DIR}/{_FILENAME}'
mocked_view_instance = MagicMock()
mocked_view_instance.view_class.pagination_class.max_page_size = None
Expand All @@ -443,12 +449,14 @@ def test_export_data_to_csv_for_default_page_size(
assert view_data['page_size'] == 100


@pytest.mark.django_db
@patch('futurex_openedx_extensions.helpers.export_csv._get_view_class_instance')
@patch('futurex_openedx_extensions.helpers.export_csv._generate_csv_with_tracked_progress')
def test_export_data_to_csv_for_missing_pagination_class(
mock_generate_csv, mock_get_view, fx_task, user
): # pylint: disable=redefined-outer-name
mock_generate_csv, mock_get_view, fx_task, base_data,
): # pylint: disable=redefined-outer-name, unused-argument
"""Test _export_data_to_csv"""
user = get_user_model().objects.get(id=30)
fake_storage_path = f'{settings.FX_DASHBOARD_STORAGE_DIR}/{_FILENAME}'
mocked_view_instance = MagicMock()
mocked_view_instance.view_class.pagination_class = None
Expand All @@ -467,11 +475,12 @@ def test_export_data_to_csv_for_missing_pagination_class(
assert view_data['page_size'] == 100


@pytest.mark.django_db
@patch('futurex_openedx_extensions.helpers.export_csv._get_view_class_instance')
@patch('futurex_openedx_extensions.helpers.export_csv._generate_csv_with_tracked_progress')
def test_export_data_to_csv_for_filename_extension(
mock_generate_csv, mock_get_view, fx_task, user
): # pylint: disable=redefined-outer-name
mock_generate_csv, mock_get_view, fx_task, base_data,
): # pylint: disable=redefined-outer-name, unused-argument
"""Test _export_data_to_csv"""
filename = 'test'
mock_view_instance = MagicMock()
Expand All @@ -480,7 +489,7 @@ def test_export_data_to_csv_for_filename_extension(
'path': 'some/path',
'query_params': {}
}
fx_permission_info = {'user_id': user.id, 'role': 'admin'}
fx_permission_info = {'user_id': 30, 'role': 'admin'}
mock_generate_csv.return_value = 'randome/path/test.csv'
export_data_to_csv(fx_task.id, 'http://example.com/api', view_data, fx_permission_info, filename)
mock_generate_csv.assert_called_once_with(
Expand All @@ -497,8 +506,8 @@ def test_export_data_to_csv_for_filename_extension(
@patch('futurex_openedx_extensions.helpers.export_csv.default_storage')
@patch('futurex_openedx_extensions.helpers.export_csv.generate_file_url')
def test_get_exported_file_url(
mocked_generate_url, mock_storage, is_file_exist, is_task_completed, expected_return_value, user, base_data
): # pylint: disable=redefined-outer-name, too-many-arguments, unused-argument
mocked_generate_url, mock_storage, is_file_exist, is_task_completed, expected_return_value, base_data,
): # pylint: disable=too-many-arguments, unused-argument
"""Test get exported file URL"""
mock_storage.exists.return_value = is_file_exist
mock_storage.url.return_value = 'http://example.com/exported_file.csv' if is_file_exist else None
Expand All @@ -507,13 +516,14 @@ def test_get_exported_file_url(
tenant_id=1,
filename='exported_file.csv',
status=DataExportTask.STATUS_COMPLETED if is_task_completed else DataExportTask.STATUS_IN_QUEUE,
user=user
user_id=30,
)
result = get_exported_file_url(task)
assert result == expected_return_value


def test_export_data_to_csv_for_url(fx_task): # pylint: disable=redefined-outer-name
@pytest.mark.django_db
def test_export_data_to_csv_for_url(base_data, fx_task): # pylint: disable=redefined-outer-name, unused-argument
"""Test _export_data_to_csv for URL"""
url_with_query_str = 'http://example.com/api?key1=value1'
with pytest.raises(FXCodedException) as exc_info:
Expand Down
Loading

0 comments on commit 8832ee1

Please sign in to comment.