From 74d72e1e69041fb3f29947e3f680bd73545dccc9 Mon Sep 17 00:00:00 2001 From: MrButtCode Date: Thu, 5 Mar 2026 11:54:39 +0500 Subject: [PATCH 1/4] Restore disabled upload controller tests and fix QueuedSampleNotFoundException - Defined missing mock_decorator helper used by login_required patch - Fixed import-time decorator issue by patching at source and reloading module - Rewrote test_link_id_confirm_valid to use HTTP test client with real session - Added default message param to QueuedSampleNotFoundException.__init__() to restore backwards compatibility with all four call sites --- exceptions.py | 11 ++- tests/test_upload/test_controllers.py | 101 ++++++++++++++++++-------- 2 files changed, 79 insertions(+), 33 deletions(-) diff --git a/exceptions.py b/exceptions.py index 658af56a..d7b805a0 100644 --- a/exceptions.py +++ b/exceptions.py @@ -3,10 +3,13 @@ class QueuedSampleNotFoundException(Exception): - """Custom exception handler for queued sample not found.""" - - def __init__(self, message: str) -> None: - Exception.__init__(self) + """Raised when a queued sample cannot be found by the given ID.""" + + def __init__(self, message="Queued sample not found."): + # By providing a default, all existing raise sites that call + # this exception without arguments will still work, and new + # calls can optionally provide a custom message. + super().__init__(message) self.message = message diff --git a/tests/test_upload/test_controllers.py b/tests/test_upload/test_controllers.py index c2a431b1..45065e39 100644 --- a/tests/test_upload/test_controllers.py +++ b/tests/test_upload/test_controllers.py @@ -2,6 +2,7 @@ from importlib import reload from io import BytesIO from unittest import mock +from functools import wraps from flask import g, url_for @@ -11,6 +12,19 @@ from tests.base import BaseTestCase, MockResponse +def mock_decorator(f): + """ + Passthrough replacement for login_required in unit tests. + + When mock.patch sets side_effect=mock_decorator, Python calls + mock_decorator(f) at decoration time and receives this wrapper, + which skips all authentication logic and delegates directly to f. + """ + @wraps(f) + def decorated_function(*args, **kwargs): + return f(*args, **kwargs) + return decorated_function + class TestControllers(BaseTestCase): """Test upload-related cases.""" @@ -170,35 +184,64 @@ def test_upload_ftp_forbidden_extension(self, mock_shutil, mock_hash, mock_magic QueuedSample.sha == filehash).first() self.assertEqual(queued_sample, None) - # TODO: The methods below are not working due to decorator login_required not being mocked - # @mock.patch('mod_upload.controllers.login_required', side_effect=mock_decorator) - # def test_link_id_confirm_invalid(self, mock_login): - # """ - # Try to confirm link for invalid sample and queue. - # """ - # from mod_upload.controllers import link_id_confirm, QueuedSampleNotFoundException - - # with self.assertRaises(QueuedSampleNotFoundException): - # link_id_confirm(1000, 1000) - - # @mock.patch('mod_upload.controllers.redirect') - # @mock.patch('mod_upload.controllers.login_required', side_effect=mock_decorator) - # @mock.patch('mod_upload.controllers.QueuedSample') - # @mock.patch('mod_upload.controllers.Sample') - # def test_link_id_confirm(self, mock_sample, mock_queue, mock_login, mock_redirect): - # """ - # Test confirm link for valid sample and queue. - # """ - # from mod_upload.controllers import link_id_confirm - - # mock_queue.query.filter.return_value.first.return_value.user_id = g.user - # mock_sample.query.filter.return_value.first.return_value.upload.user_id = g.user - - # response = link_id_confirm(1, 1) - - # self.assertEqual(response, mock_redirect()) - # mock_queue.query.filter.assert_called_once_with(mock_queue.id == 1) - # mock_sample.query.filter.assert_called_once_with(mock_sample.id == 1) + @mock.patch('mod_auth.controllers.login_required', side_effect=mock_decorator) + def test_link_id_confirm_invalid(self, mock_login): + """Confirm that an unrecognised upload/sample pair raises QueuedSampleNotFoundException.""" + # Patching the source module and reloading the consumer forces all + # @login_required decorators in mod_upload.controllers to be re-applied + # under the mock, since decorators are evaluated at import time. + import mod_upload.controllers + reload(mod_upload.controllers) + + from mod_upload.controllers import link_id_confirm, QueuedSampleNotFoundException + with self.assertRaises(QueuedSampleNotFoundException): + link_id_confirm(1000, 1000) + + def test_link_id_confirm_valid(self): + """ + Test that a logged-in user can confirm a link between a queued upload + and an existing sample when they own both records. + + The controller traverses a Sample -> Upload -> User ownership chain, + so the test database must contain the full relational structure before + hitting the route. A bare QueuedSample is not sufficient. + """ + from mod_auth.models import User + from mod_sample.models import Sample + from mod_upload.models import Platform, QueuedSample, Upload + self.create_user_with_role( + self.user.name, self.user.email, self.user.password, Role.user) + # create_user_with_role has no return statement, so we query by the + # unique email field to get the SQLAlchemy instance with its real primary key. + db_user = User.query.filter(User.email == self.user.email).first() + + with self.app.test_client() as c: + c.post('/account/login', data=self.create_login_form_data( + self.user.email, self.user.password)) + + filehash = 'confirm_test_hash' + + # The controller checks sample.upload.user_id == g.user.id, which means + # Sample and Upload must exist and be correctly linked before the request. + test_sample = Sample(filehash, '.ts', f'{filehash}.ts') + g.db.add(test_sample) + g.db.flush() # flush before Upload so test_sample.id is populated + + # Upload is the bridge between User and Sample; version_id=1 assumes + # the test database seeds a default CCExtractorVersion row. + test_upload = Upload(db_user.id, test_sample.id, 1, Platform.linux) + g.db.add(test_upload) + g.db.flush() # flush before QueuedSample so test_upload.id is populated + + queued_sample = QueuedSample(filehash, '.ts', f'{filehash}.ts', db_user.id) + g.db.add(queued_sample) + g.db.commit() + + # Pass both dynamically generated IDs so the route resolves correctly + # regardless of insertion order across test runs. + response = c.get(f'/upload/link/{queued_sample.id}/{test_sample.id}') + + self.assertEqual(response.status_code, 302) def test_create_hash_for_sample(self): """Test creating hash for temp file.""" From b29efe9f8beb2cffdddff73f2d3ad8b93de28815 Mon Sep 17 00:00:00 2001 From: MrButtCode Date: Fri, 6 Mar 2026 15:58:06 +0500 Subject: [PATCH 2/4] refactor: revert to unit test style using unwrapped and remove verbose comments --- tests/test_upload/test_controllers.py | 73 ++++++++------------------- 1 file changed, 21 insertions(+), 52 deletions(-) diff --git a/tests/test_upload/test_controllers.py b/tests/test_upload/test_controllers.py index 45065e39..1c296d64 100644 --- a/tests/test_upload/test_controllers.py +++ b/tests/test_upload/test_controllers.py @@ -184,64 +184,33 @@ def test_upload_ftp_forbidden_extension(self, mock_shutil, mock_hash, mock_magic QueuedSample.sha == filehash).first() self.assertEqual(queued_sample, None) - @mock.patch('mod_auth.controllers.login_required', side_effect=mock_decorator) - def test_link_id_confirm_invalid(self, mock_login): - """Confirm that an unrecognised upload/sample pair raises QueuedSampleNotFoundException.""" - # Patching the source module and reloading the consumer forces all - # @login_required decorators in mod_upload.controllers to be re-applied - # under the mock, since decorators are evaluated at import time. - import mod_upload.controllers - reload(mod_upload.controllers) - + def test_link_id_confirm_invalid(self): + """Try to confirm link for invalid sample and queue.""" from mod_upload.controllers import link_id_confirm, QueuedSampleNotFoundException - with self.assertRaises(QueuedSampleNotFoundException): - link_id_confirm(1000, 1000) - def test_link_id_confirm_valid(self): - """ - Test that a logged-in user can confirm a link between a queued upload - and an existing sample when they own both records. - - The controller traverses a Sample -> Upload -> User ownership chain, - so the test database must contain the full relational structure before - hitting the route. A bare QueuedSample is not sufficient. - """ - from mod_auth.models import User - from mod_sample.models import Sample - from mod_upload.models import Platform, QueuedSample, Upload - self.create_user_with_role( - self.user.name, self.user.email, self.user.password, Role.user) - # create_user_with_role has no return statement, so we query by the - # unique email field to get the SQLAlchemy instance with its real primary key. - db_user = User.query.filter(User.email == self.user.email).first() + with self.assertRaises(QueuedSampleNotFoundException): + link_id_confirm.__wrapped__(1000, 1000) - with self.app.test_client() as c: - c.post('/account/login', data=self.create_login_form_data( - self.user.email, self.user.password)) - - filehash = 'confirm_test_hash' - - # The controller checks sample.upload.user_id == g.user.id, which means - # Sample and Upload must exist and be correctly linked before the request. - test_sample = Sample(filehash, '.ts', f'{filehash}.ts') - g.db.add(test_sample) - g.db.flush() # flush before Upload so test_sample.id is populated + @mock.patch('mod_upload.controllers.redirect') + @mock.patch('mod_upload.controllers.QueuedSample') + @mock.patch('mod_upload.controllers.Sample') + @mock.patch('mod_upload.controllers.g') + def test_link_id_confirm_valid(self, mock_g, mock_sample, mock_queue, mock_redirect): + """Test confirm link for valid sample and queue.""" + from mod_upload.controllers import link_id_confirm - # Upload is the bridge between User and Sample; version_id=1 assumes - # the test database seeds a default CCExtractorVersion row. - test_upload = Upload(db_user.id, test_sample.id, 1, Platform.linux) - g.db.add(test_upload) - g.db.flush() # flush before QueuedSample so test_upload.id is populated + # Mock g.user to have an 'id' attribute to satisfy the controller + mock_g.user.id = 1 + + # Ensure the mocked database queries return an object owned by that user + mock_queue.query.filter.return_value.first.return_value.user_id = mock_g.user.id + mock_sample.query.filter.return_value.first.return_value.upload.user_id = mock_g.user.id - queued_sample = QueuedSample(filehash, '.ts', f'{filehash}.ts', db_user.id) - g.db.add(queued_sample) - g.db.commit() + response = link_id_confirm.__wrapped__(1, 1) - # Pass both dynamically generated IDs so the route resolves correctly - # regardless of insertion order across test runs. - response = c.get(f'/upload/link/{queued_sample.id}/{test_sample.id}') - - self.assertEqual(response.status_code, 302) + self.assertEqual(response, mock_redirect()) + mock_queue.query.filter.assert_called_once() + mock_sample.query.filter.assert_called_once() def test_create_hash_for_sample(self): """Test creating hash for temp file.""" From 6eeb8224a3d3a6905f01c4e016f703c4967a4d7a Mon Sep 17 00:00:00 2001 From: MrButtCode Date: Sat, 7 Mar 2026 15:40:44 +0500 Subject: [PATCH 3/4] style: auto-format imports with isort to resolve CI failure --- tests/test_upload/test_controllers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_upload/test_controllers.py b/tests/test_upload/test_controllers.py index 1c296d64..fe3ee8ce 100644 --- a/tests/test_upload/test_controllers.py +++ b/tests/test_upload/test_controllers.py @@ -1,8 +1,8 @@ import base64 +from functools import wraps from importlib import reload from io import BytesIO from unittest import mock -from functools import wraps from flask import g, url_for @@ -186,7 +186,8 @@ def test_upload_ftp_forbidden_extension(self, mock_shutil, mock_hash, mock_magic def test_link_id_confirm_invalid(self): """Try to confirm link for invalid sample and queue.""" - from mod_upload.controllers import link_id_confirm, QueuedSampleNotFoundException + from mod_upload.controllers import (QueuedSampleNotFoundException, + link_id_confirm) with self.assertRaises(QueuedSampleNotFoundException): link_id_confirm.__wrapped__(1000, 1000) From a7588ef5b6accf4746e479e83ea3e320bfe0e886 Mon Sep 17 00:00:00 2001 From: MrButtCode Date: Sun, 8 Mar 2026 13:09:05 +0500 Subject: [PATCH 4/4] refactor: remove orphaned mock_decorator and unused imports --- tests/test_upload/test_controllers.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/test_upload/test_controllers.py b/tests/test_upload/test_controllers.py index fe3ee8ce..fb2dd389 100644 --- a/tests/test_upload/test_controllers.py +++ b/tests/test_upload/test_controllers.py @@ -1,5 +1,4 @@ import base64 -from functools import wraps from importlib import reload from io import BytesIO from unittest import mock @@ -12,19 +11,6 @@ from tests.base import BaseTestCase, MockResponse -def mock_decorator(f): - """ - Passthrough replacement for login_required in unit tests. - - When mock.patch sets side_effect=mock_decorator, Python calls - mock_decorator(f) at decoration time and receives this wrapper, - which skips all authentication logic and delegates directly to f. - """ - @wraps(f) - def decorated_function(*args, **kwargs): - return f(*args, **kwargs) - return decorated_function - class TestControllers(BaseTestCase): """Test upload-related cases."""