From 3b7b8c4842b77c62dd30eec2427818297f619fb7 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Thu, 18 Dec 2025 16:39:37 +0100 Subject: [PATCH 01/12] refactor: split out login test into separate file --- src/webview/tests/test_login.py | 50 +++++++++++++++++++++++++++ src/webview/tests/test_suggestions.py | 49 -------------------------- 2 files changed, 50 insertions(+), 49 deletions(-) create mode 100644 src/webview/tests/test_login.py diff --git a/src/webview/tests/test_login.py b/src/webview/tests/test_login.py new file mode 100644 index 00000000..6ea924c7 --- /dev/null +++ b/src/webview/tests/test_login.py @@ -0,0 +1,50 @@ +from typing import Any +from unittest.mock import patch + +from allauth.account.utils import get_login_redirect_url +from allauth.socialaccount.providers.oauth2.views import OAuth2LoginView +from allauth.socialaccount.templatetags.socialaccount import provider_login_url +from django.contrib.auth import get_user_model, login +from django.http import HttpRequest, HttpResponse +from django.shortcuts import redirect +from django.test import Client +from django.urls import reverse + + +def test_redirect_after_login(db: None, client: Client) -> None: + # location to start navigation + entry_point = reverse("webview:suggestions_view") + response = client.get(entry_point) + # expect the correct login URL in the response, + # which will redirect back to the same page on success + context = response.context[0] + login_url = provider_login_url( + context, + provider="github", + next=context.request.get_full_path(), + ) + assert login_url in response.content.decode("utf-8") + # follow the login workflow as displayed + response = client.get(login_url) + # expect confirmation to log in and a CSRF barrier via POST request + assert "form" in response.content.decode("utf-8") + + def mock_login( + self: OAuth2LoginView, request: HttpRequest, *args: Any, **kwargs: Any + ) -> HttpResponse: + user, _ = get_user_model().objects.get_or_create(username="testuser") + login( + request, + user, + backend="allauth.account.auth_backends.AuthenticationBackend", + ) + return redirect(get_login_redirect_url(request)) + + with patch.object(OAuth2LoginView, "dispatch", mock_login): + # log in and follow redirect + response = client.post(login_url, {}, follow=True) + + assert response.status_code == 200 + assert response.context["user"].is_authenticated + location, status = response.redirect_chain[-1] + assert location == entry_point diff --git a/src/webview/tests/test_suggestions.py b/src/webview/tests/test_suggestions.py index d7c69d5a..09335156 100644 --- a/src/webview/tests/test_suggestions.py +++ b/src/webview/tests/test_suggestions.py @@ -1,15 +1,8 @@ from datetime import timedelta -from typing import Any from unittest.mock import patch -from allauth.account.utils import get_login_redirect_url from allauth.socialaccount.models import SocialAccount -from allauth.socialaccount.providers.oauth2.views import OAuth2LoginView -from allauth.socialaccount.templatetags.socialaccount import provider_login_url -from django.contrib.auth import get_user_model, login from django.contrib.auth.models import User -from django.http import HttpRequest, HttpResponse -from django.shortcuts import redirect from django.test import Client, TestCase from django.urls import reverse from django.utils import timezone @@ -259,48 +252,6 @@ def test_restore_package(self) -> None: self.assertIn("package2", cached_packages) -class Login(TestCase): - def setUp(self) -> None: - # location to start navigation - self.entry_point = reverse("webview:suggestions_view") - - def test_redirect_after_login(self) -> None: - response = self.client.get(self.entry_point) - # expect the correct login URL in the response, - # which will redirect back to the same page on success - context = response.context[0] - login_url = provider_login_url( - context, - provider="github", - next=context.request.get_full_path(), - ) - self.assertIn(login_url, response.content.decode("utf-8")) - # follow the login workflow as displayed - response = self.client.get(login_url) - # expect confirmation to log in and a CSRF barrier via POST request - self.assertIn("form", response.content.decode("utf-8")) - - def mock_login( - self: OAuth2LoginView, request: HttpRequest, *args: Any, **kwargs: Any - ) -> HttpResponse: - user, _ = get_user_model().objects.get_or_create(username="testuser") - login( - request, - user, - backend="allauth.account.auth_backends.AuthenticationBackend", - ) - return redirect(get_login_redirect_url(request)) - - with patch.object(OAuth2LoginView, "dispatch", mock_login): - # log in and follow redirect - response = self.client.post(login_url, {}, follow=True) - - self.assertEqual(response.status_code, 200) - self.assertTrue(response.context["user"].is_authenticated) - location, status = response.redirect_chain[-1] - self.assertEqual(location, self.entry_point) - - class AddMaintainerViewTests(TestCase): def setUp(self) -> None: # Create user and log in From 6a11101a70ac732c98ce123cd57333d1a3b1e7d8 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 12:54:41 +0100 Subject: [PATCH 02/12] fix: linked CVE in fixture not sure why it worked before... --- src/shared/tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/tests/conftest.py b/src/shared/tests/conftest.py index f334564a..6eb72475 100644 --- a/src/shared/tests/conftest.py +++ b/src/shared/tests/conftest.py @@ -96,7 +96,7 @@ def drv(db: None, evaluation: NixEvaluation) -> NixDerivation: def suggestion(cve: Container, drv: NixDerivation) -> CVEDerivationClusterProposal: suggestion = CVEDerivationClusterProposal.objects.create( status="pending", - cve_id=cve.pk, + cve=cve.cve, ) DerivationClusterProposalLink.objects.create( From 3b5657ce076ba5edc0243be00760a930bd23b729 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 23:03:04 +0100 Subject: [PATCH 03/12] chore: make fixtures reusable accross apps --- src/shared/tests/conftest.py | 109 +--------------------------------- src/shared/tests/fixtures.py | 108 +++++++++++++++++++++++++++++++++ src/webview/tests/conftest.py | 1 + 3 files changed, 110 insertions(+), 108 deletions(-) create mode 100644 src/shared/tests/fixtures.py create mode 100644 src/webview/tests/conftest.py diff --git a/src/shared/tests/conftest.py b/src/shared/tests/conftest.py index 6eb72475..27dfac93 100644 --- a/src/shared/tests/conftest.py +++ b/src/shared/tests/conftest.py @@ -1,108 +1 @@ -import pytest - -from shared.models.cve import ( - AffectedProduct, - Container, - CveRecord, - Description, - Metric, - Organization, - Version, -) -from shared.models.linkage import ( - CVEDerivationClusterProposal, - DerivationClusterProposalLink, - ProvenanceFlags, -) -from shared.models.nix_evaluation import ( - NixChannel, - NixDerivation, - NixDerivationMeta, - NixEvaluation, - NixMaintainer, -) - - -@pytest.fixture -def cve(db: None) -> Container: - org = Organization.objects.create(uuid=1, short_name="test-org") - cve = CveRecord.objects.create( - cve_id="CVE-2025-0001", - assigner=org, - ) - desc = Description.objects.create(value="Test description") - metric = Metric.objects.create(format="cvssV3_1", raw_cvss_json={}) - version = Version.objects.create(status=Version.Status.AFFECTED, version="1.0") - affected = AffectedProduct.objects.create(package_name="dummy-package") - affected.versions.add(version) - - container = cve.container.create(provider=org, title="Dummy Title") - container.affected.add(affected) - container.descriptions.add(desc) - container.metrics.add(metric) - - return container - - -@pytest.fixture -def evaluation(db: None) -> NixEvaluation: - channel = NixChannel.objects.create( - staging_branch="release-24.05", - channel_branch="release-24.05", - head_sha1_commit="deadbeef", - state=NixChannel.ChannelState.STABLE, - release_version="24.05", - repository="https://github.com/NixOS/nixpkgs", - ) - - evaluation = NixEvaluation.objects.create( - channel=channel, - commit_sha1="deadbeef", - state="completed", - ) - return evaluation - - -@pytest.fixture -def drv(db: None, evaluation: NixEvaluation) -> NixDerivation: - maintainer = NixMaintainer.objects.create( - github_id=123, github="testuser", name="Test User", email="test@example.com" - ) - - meta = NixDerivationMeta.objects.create( - description="First dummy derivation", - insecure=False, - available=True, - broken=False, - unfree=False, - unsupported=False, - ) - - meta.maintainers.add(maintainer) - - drv = NixDerivation.objects.create( - attribute="foo", - derivation_path="/nix/store/-foo.drv", - name="foo-1.0", - metadata=meta, - system="x86_64-linux", - parent_evaluation=evaluation, - ) - - return drv - - -@pytest.fixture -def suggestion(cve: Container, drv: NixDerivation) -> CVEDerivationClusterProposal: - suggestion = CVEDerivationClusterProposal.objects.create( - status="pending", - cve=cve.cve, - ) - - DerivationClusterProposalLink.objects.create( - proposal=suggestion, - derivation=drv, - provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, - ) - - return suggestion +pytest_plugins = ["shared.tests.fixtures"] diff --git a/src/shared/tests/fixtures.py b/src/shared/tests/fixtures.py new file mode 100644 index 00000000..6eb72475 --- /dev/null +++ b/src/shared/tests/fixtures.py @@ -0,0 +1,108 @@ +import pytest + +from shared.models.cve import ( + AffectedProduct, + Container, + CveRecord, + Description, + Metric, + Organization, + Version, +) +from shared.models.linkage import ( + CVEDerivationClusterProposal, + DerivationClusterProposalLink, + ProvenanceFlags, +) +from shared.models.nix_evaluation import ( + NixChannel, + NixDerivation, + NixDerivationMeta, + NixEvaluation, + NixMaintainer, +) + + +@pytest.fixture +def cve(db: None) -> Container: + org = Organization.objects.create(uuid=1, short_name="test-org") + cve = CveRecord.objects.create( + cve_id="CVE-2025-0001", + assigner=org, + ) + desc = Description.objects.create(value="Test description") + metric = Metric.objects.create(format="cvssV3_1", raw_cvss_json={}) + version = Version.objects.create(status=Version.Status.AFFECTED, version="1.0") + affected = AffectedProduct.objects.create(package_name="dummy-package") + affected.versions.add(version) + + container = cve.container.create(provider=org, title="Dummy Title") + container.affected.add(affected) + container.descriptions.add(desc) + container.metrics.add(metric) + + return container + + +@pytest.fixture +def evaluation(db: None) -> NixEvaluation: + channel = NixChannel.objects.create( + staging_branch="release-24.05", + channel_branch="release-24.05", + head_sha1_commit="deadbeef", + state=NixChannel.ChannelState.STABLE, + release_version="24.05", + repository="https://github.com/NixOS/nixpkgs", + ) + + evaluation = NixEvaluation.objects.create( + channel=channel, + commit_sha1="deadbeef", + state="completed", + ) + return evaluation + + +@pytest.fixture +def drv(db: None, evaluation: NixEvaluation) -> NixDerivation: + maintainer = NixMaintainer.objects.create( + github_id=123, github="testuser", name="Test User", email="test@example.com" + ) + + meta = NixDerivationMeta.objects.create( + description="First dummy derivation", + insecure=False, + available=True, + broken=False, + unfree=False, + unsupported=False, + ) + + meta.maintainers.add(maintainer) + + drv = NixDerivation.objects.create( + attribute="foo", + derivation_path="/nix/store/-foo.drv", + name="foo-1.0", + metadata=meta, + system="x86_64-linux", + parent_evaluation=evaluation, + ) + + return drv + + +@pytest.fixture +def suggestion(cve: Container, drv: NixDerivation) -> CVEDerivationClusterProposal: + suggestion = CVEDerivationClusterProposal.objects.create( + status="pending", + cve=cve.cve, + ) + + DerivationClusterProposalLink.objects.create( + proposal=suggestion, + derivation=drv, + provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, + ) + + return suggestion diff --git a/src/webview/tests/conftest.py b/src/webview/tests/conftest.py new file mode 100644 index 00000000..27dfac93 --- /dev/null +++ b/src/webview/tests/conftest.py @@ -0,0 +1 @@ +pytest_plugins = ["shared.tests.fixtures"] From 5729c12439cc3429b5c3bc92e7702819731e78ad Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 14:17:12 +0100 Subject: [PATCH 04/12] refactor: migate testing publishing issues to `pytest` fixtures (1/2) --- src/shared/tests/fixtures.py | 18 +++++++ src/webview/tests/test_issues.py | 85 +++++++++++++++++--------------- 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/src/shared/tests/fixtures.py b/src/shared/tests/fixtures.py index 6eb72475..91529836 100644 --- a/src/shared/tests/fixtures.py +++ b/src/shared/tests/fixtures.py @@ -1,4 +1,6 @@ import pytest +from django.contrib.auth.models import AbstractBaseUser +from django.test import Client from shared.models.cve import ( AffectedProduct, @@ -106,3 +108,19 @@ def suggestion(cve: Container, drv: NixDerivation) -> CVEDerivationClusterPropos ) return suggestion + + +@pytest.fixture +def authenticated_client( + client: Client, django_user_model: type[AbstractBaseUser] +) -> Client: + user = django_user_model.objects.create_user( + username="testuser", + is_staff=True, + ) + # https://docs.djangoproject.com/en/6.0/topics/testing/tools/#django.test.Client.force_login + client.force_login( + user, + backend="allauth.account.auth_backends.AuthenticationBackend", + ) + return client diff --git a/src/webview/tests/test_issues.py b/src/webview/tests/test_issues.py index 947a43b1..5ad37ea6 100644 --- a/src/webview/tests/test_issues.py +++ b/src/webview/tests/test_issues.py @@ -10,6 +10,7 @@ from shared.listeners.cache_suggestions import cache_new_suggestions from shared.models.cve import ( AffectedProduct, + Container, CveRecord, Description, Metric, @@ -31,6 +32,49 @@ from shared.tests.test_github_sync import MockGithub +def test_publish_gh_issue_empty_title( + db: None, + cve: Container, + suggestion: CVEDerivationClusterProposal, + authenticated_client: Client, +) -> None: + """Test that creating a GitHub issue will succeed and update the suggestion status, despite empty CVE title""" + # [tag:test-github-create_issue-title] + + url = reverse("webview:drafts_view") + # 3/4 of all CVEs in the source data have empty title + cve.title = "" + cve.save() + suggestion.status = CVEDerivationClusterProposal.Status.ACCEPTED + suggestion.save() + cache_new_suggestions(suggestion) + + # FIXME(@fricklerhandwerk): Mock Github's `create_issue()` here, not our own procedure! [ref:todo-github-connection] + # Then we can test in-context that the right arguments have been passed, using `mock.assert_called_with()`. + with patch("webview.views.create_gh_issue") as mock: + mock.side_effect = lambda *args, **kwargs: create_gh_issue( + *args, + github=MockGithub(expected_issue_title="Test description"), # type: ignore + **kwargs, + ) + response = authenticated_client.post( + url, + { + "suggestion_id": suggestion.pk, + "new_status": "published", + "comment": "", # Empty comment + }, + ) + mock.assert_called() + + messages = list(get_messages(response.wsgi_request)) + assert not any(m.level_tag == "error" for m in messages), ( + "Errors on issue submission" + ) + suggestion.refresh_from_db() + assert suggestion.status == CVEDerivationClusterProposal.Status.PUBLISHED + + class IssueTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -126,47 +170,6 @@ def setUp(self) -> None: cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - def test_publish_gh_issue_empty_title(self) -> None: - """Test that creating a GitHub issue will succeed and update the suggestion status, despite empty CVE title""" - # [tag:test-github-create_issue-title] - - url = reverse("webview:drafts_view") - # 3/4 of all CVEs in the source data have empty title - self.cve_container.title = "" - self.cve_container.save() - self.suggestion.status = CVEDerivationClusterProposal.Status.ACCEPTED - self.suggestion.save() - cache_new_suggestions(self.suggestion) - - # FIXME(@fricklerhandwerk): Mock Github's `create_issue()` here, not our own procedure! [ref:todo-github-connection] - # Then we can test in-context that the right arguments have been passed, using `mock.assert_called_with()`. - with patch("webview.views.create_gh_issue") as mock: - mock.side_effect = lambda *args, **kwargs: create_gh_issue( - *args, - github=MockGithub(expected_issue_title="Test description"), # type: ignore - **kwargs, - ) - - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "published", - "comment": "", # Empty comment - }, - ) - - messages = list(get_messages(response.wsgi_request)) - self.assertFalse( - any(m.level_tag == "error" for m in messages), - "Errors on issue submission", - ) - self.suggestion.refresh_from_db() - self.assertEqual( - self.suggestion.status, - CVEDerivationClusterProposal.Status.PUBLISHED, - ) - def test_publish_gh_issue_empty_description(self) -> None: """Test that creating a GitHub issue will succeed and update suggestion status, despite no CVE description""" # [tag:test-github-create_issue-description] From 7c0a6a709217f5bc1da952786377dd6091f4041c Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 14:18:56 +0100 Subject: [PATCH 05/12] refactor: migate testing publishing issues to `pytest` fixtures (2/2) --- src/webview/tests/test_issues.py | 80 ++++++++++++++++---------------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/src/webview/tests/test_issues.py b/src/webview/tests/test_issues.py index 5ad37ea6..920b0e46 100644 --- a/src/webview/tests/test_issues.py +++ b/src/webview/tests/test_issues.py @@ -75,6 +75,47 @@ def test_publish_gh_issue_empty_title( assert suggestion.status == CVEDerivationClusterProposal.Status.PUBLISHED +def test_publish_gh_issue_empty_description( + db: None, + cve: Container, + suggestion: CVEDerivationClusterProposal, + authenticated_client: Client, +) -> None: + """Test that creating a GitHub issue will succeed and update suggestion status, despite no CVE description""" + # [tag:test-github-create_issue-description] + + url = reverse("webview:drafts_view") + cve.descriptions.clear() + cve.save() + suggestion.status = CVEDerivationClusterProposal.Status.ACCEPTED + suggestion.save() + cache_new_suggestions(suggestion) + + # FIXME(@fricklerhandwerk): Mock Github's `create_issue()` here, not our own procedure! [ref:todo-github-connection] + with patch("webview.views.create_gh_issue") as mock: + mock.side_effect = lambda *args, **kwargs: create_gh_issue( + *args, + github=MockGithub(expected_issue_title="Dummy Title"), # type: ignore + **kwargs, + ) + response = authenticated_client.post( + url, + { + "suggestion_id": suggestion.pk, + "new_status": "published", + "comment": "", # Empty comment + }, + ) + mock.assert_called() + + messages = list(get_messages(response.wsgi_request)) + assert not any(m.level_tag == "error" for m in messages), ( + "Errors on issue submission" + ) + suggestion.refresh_from_db() + assert suggestion.status == CVEDerivationClusterProposal.Status.PUBLISHED + + class IssueTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -169,42 +210,3 @@ def setUp(self) -> None: # Cache the suggestion cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - - def test_publish_gh_issue_empty_description(self) -> None: - """Test that creating a GitHub issue will succeed and update suggestion status, despite no CVE description""" - # [tag:test-github-create_issue-description] - - url = reverse("webview:drafts_view") - self.cve_container.descriptions.clear() - self.cve_container.save() - self.suggestion.status = CVEDerivationClusterProposal.Status.ACCEPTED - self.suggestion.save() - cache_new_suggestions(self.suggestion) - - # FIXME(@fricklerhandwerk): Mock Github's `create_issue()` here, not our own procedure! [ref:todo-github-connection] - with patch("webview.views.create_gh_issue") as mock: - mock.side_effect = lambda *args, **kwargs: create_gh_issue( - *args, - github=MockGithub(expected_issue_title="Dummy Title"), # type: ignore - **kwargs, - ) - - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "published", - "comment": "", # Empty comment - }, - ) - - messages = list(get_messages(response.wsgi_request)) - self.assertFalse( - any(m.level_tag == "error" for m in messages), - "Errors on issue submission", - ) - self.suggestion.refresh_from_db() - self.assertEqual( - self.suggestion.status, - CVEDerivationClusterProposal.Status.PUBLISHED, - ) From 49be3bc37d4172e9e52969592b950619a08ce03c Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 14:19:33 +0100 Subject: [PATCH 06/12] refactor: remove unused `TestCase` setup --- src/webview/tests/test_issues.py | 115 +------------------------------ 1 file changed, 1 insertion(+), 114 deletions(-) diff --git a/src/webview/tests/test_issues.py b/src/webview/tests/test_issues.py index 920b0e46..cfeafe1b 100644 --- a/src/webview/tests/test_issues.py +++ b/src/webview/tests/test_issues.py @@ -1,33 +1,16 @@ from unittest.mock import patch -from allauth.socialaccount.models import SocialAccount -from django.contrib.auth.models import User from django.contrib.messages import get_messages -from django.test import Client, TestCase +from django.test import Client from django.urls import reverse from shared.github import create_gh_issue from shared.listeners.cache_suggestions import cache_new_suggestions from shared.models.cve import ( - AffectedProduct, Container, - CveRecord, - Description, - Metric, - Organization, - Version, ) from shared.models.linkage import ( CVEDerivationClusterProposal, - DerivationClusterProposalLink, - ProvenanceFlags, -) -from shared.models.nix_evaluation import ( - NixChannel, - NixDerivation, - NixDerivationMeta, - NixEvaluation, - NixMaintainer, ) from shared.tests.test_github_sync import MockGithub @@ -114,99 +97,3 @@ def test_publish_gh_issue_empty_description( ) suggestion.refresh_from_db() assert suggestion.status == CVEDerivationClusterProposal.Status.PUBLISHED - - -class IssueTests(TestCase): - def setUp(self) -> None: - # Create user and log in - self.user = User.objects.create_user(username="admin", password="pw") - self.user.is_staff = True - self.user.save() - - # Create a GitHub social account for the user - SocialAccount.objects.get_or_create( - user=self.user, - provider="github", - uid="123456", - extra_data={"login": "admin"}, - ) - - self.client = Client() - self.client.login(username="admin", password="pw") - - # Create CVE and related objects - self.assigner = Organization.objects.create(uuid=1, short_name="foo") - self.cve_record = CveRecord.objects.create( - cve_id="CVE-2025-0001", - assigner=self.assigner, - ) - self.description = Description.objects.create(value="Test description") - self.metric = Metric.objects.create(format="cvssV3_1", raw_cvss_json={}) - self.affected_product = AffectedProduct.objects.create( - package_name="dummy-package" - ) - self.affected_product.versions.add( - Version.objects.create(status=Version.Status.AFFECTED, version="1.0") - ) - self.cve_container = self.cve_record.container.create( - provider=self.assigner, - title="Dummy Title", - ) - self.cve_container.affected.add(self.affected_product) - self.cve_container.descriptions.add(self.description) - self.cve_container.metrics.add(self.metric) - - # Create maintainer and metadata - self.maintainer = NixMaintainer.objects.create( - github_id=123, - github="testuser", - name="Test User", - email="test@example.com", - ) - self.meta = NixDerivationMeta.objects.create( - description="Dummy derivation", - insecure=False, - available=True, - broken=False, - unfree=False, - unsupported=False, - ) - self.meta.maintainers.add(self.maintainer) - - # Create evaluation and derivation - self.evaluation = NixEvaluation.objects.create( - channel=NixChannel.objects.create( - staging_branch="release-24.05", - channel_branch="nixos-24.05", - head_sha1_commit="deadbeef", - state=NixChannel.ChannelState.STABLE, - release_version="24.05", - repository="https://github.com/NixOS/nixpkgs", - ), - commit_sha1="deadbeef", - state=NixEvaluation.EvaluationState.COMPLETED, - ) - - self.derivation = NixDerivation.objects.create( - attribute="package1", - derivation_path="/nix/store/package1.drv", - name="package1-1.0", - metadata=self.meta, - system="x86_64-linux", - parent_evaluation=self.evaluation, - ) - - # Create suggestion and link derivation - self.suggestion = CVEDerivationClusterProposal.objects.create( - status=CVEDerivationClusterProposal.Status.PENDING, - cve_id=self.cve_record.pk, - ) - DerivationClusterProposalLink.objects.create( - proposal=self.suggestion, - derivation=self.derivation, - provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, - ) - - # Cache the suggestion - cache_new_suggestions(self.suggestion) - self.suggestion.refresh_from_db() From cd13b285cdc5931bcd6df2bc2a034eb84803e83a Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 13:26:24 +0100 Subject: [PATCH 07/12] refactor: migate testing mandatory comments to `pytest` fixtures (1/6) --- src/shared/tests/fixtures.py | 17 +++++++++++ src/webview/tests/test_comments.py | 47 ++++++++++++++++-------------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/shared/tests/fixtures.py b/src/shared/tests/fixtures.py index 91529836..8ffc6ad0 100644 --- a/src/shared/tests/fixtures.py +++ b/src/shared/tests/fixtures.py @@ -1,7 +1,9 @@ import pytest +from allauth.socialaccount.models import SocialAccount from django.contrib.auth.models import AbstractBaseUser from django.test import Client +from shared.listeners.cache_suggestions import cache_new_suggestions from shared.models.cve import ( AffectedProduct, Container, @@ -110,6 +112,15 @@ def suggestion(cve: Container, drv: NixDerivation) -> CVEDerivationClusterPropos return suggestion +@pytest.fixture +def cached_suggestion( + suggestion: CVEDerivationClusterProposal, +) -> CVEDerivationClusterProposal: + cache_new_suggestions(suggestion) + + return suggestion + + @pytest.fixture def authenticated_client( client: Client, django_user_model: type[AbstractBaseUser] @@ -118,6 +129,12 @@ def authenticated_client( username="testuser", is_staff=True, ) + SocialAccount.objects.get_or_create( + user=user, + provider="github", + uid="123456", + extra_data={"login": user.username}, + ) # https://docs.djangoproject.com/en/6.0/topics/testing/tools/#django.test.Client.force_login client.force_login( user, diff --git a/src/webview/tests/test_comments.py b/src/webview/tests/test_comments.py index 300cf72c..21cd3a57 100644 --- a/src/webview/tests/test_comments.py +++ b/src/webview/tests/test_comments.py @@ -27,6 +27,31 @@ ) +def test_dismiss_requires_comment_htmx( + db: None, + authenticated_client: Client, + cached_suggestion: CVEDerivationClusterProposal, +) -> None: + """Test that dismissing a suggestion requires a comment (HTMX case)""" + url = reverse("webview:suggestions_view") + + # Try to dismiss without a comment using HTMX + response = authenticated_client.post( + url, + { + "suggestion_id": cached_suggestion.pk, + "new_status": "rejected", + "comment": "", # Empty comment + }, + HTTP_HX_REQUEST="true", # Simulate HTMX request + ) + + # Should return 200 with error_message in context for HTMX + assert response.status_code == 200 + assert "error_message" in response.context + assert response.context["error_message"] == "You must provide a dismissal comment" + + class CommentTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -122,28 +147,6 @@ def setUp(self) -> None: cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - def test_dismiss_requires_comment_htmx(self) -> None: - """Test that dismissing a suggestion requires a comment (HTMX case)""" - url = reverse("webview:suggestions_view") - - # Try to dismiss without a comment using HTMX - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "rejected", - "comment": "", # Empty comment - }, - HTTP_HX_REQUEST="true", # Simulate HTMX request - ) - - # Should return 200 with error_message in context for HTMX - self.assertEqual(response.status_code, 200) - self.assertIn("error_message", response.context) - self.assertEqual( - response.context["error_message"], "You must provide a dismissal comment" - ) - def test_dismiss_requires_comment_no_js(self) -> None: """Test that dismissing a suggestion requires a comment (no-JS case)""" url = reverse("webview:suggestions_view") From 948a77b613bc651fc037566d4d0c45a531b173d7 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 13:29:22 +0100 Subject: [PATCH 08/12] refactor: migate testing mandatory comments to `pytest` fixtures (2/6) --- src/webview/tests/test_comments.py | 73 +++++++++++++++--------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/src/webview/tests/test_comments.py b/src/webview/tests/test_comments.py index 21cd3a57..f64ba3c9 100644 --- a/src/webview/tests/test_comments.py +++ b/src/webview/tests/test_comments.py @@ -52,6 +52,43 @@ def test_dismiss_requires_comment_htmx( assert response.context["error_message"] == "You must provide a dismissal comment" +def test_dismiss_requires_comment_no_js( + authenticated_client: Client, cached_suggestion: CVEDerivationClusterProposal +) -> None: + """Test that dismissing a suggestion requires a comment (no-JS case)""" + url = reverse("webview:suggestions_view") + + # Try to dismiss without a comment (non-JS behavior) + response = authenticated_client.post( + url, + { + "suggestion_id": cached_suggestion.pk, + "new_status": "rejected", + "comment": "", # Empty comment + "no-js": "", # Indicate non-JS mode + }, + ) + + # Should redirect back to the same page + assert response.status_code == 302 + + # Follow the redirect and check for Django messages + follow_response = authenticated_client.get(url) + assert follow_response.status_code == 200 + + # Check that the correct error message was added to Django messages + messages = list(get_messages(follow_response.wsgi_request)) + assert len(messages) == 1 + assert str(messages[0]) == "You must provide a dismissal comment" + + # Verify the suggestion is still in the suggestions view (not dismissed) + suggestions = follow_response.context["object_list"] + our_suggestion = next( + (s for s in suggestions if s.proposal_id == cached_suggestion.pk), None + ) + assert our_suggestion is not None, "Suggestion should still be in pending status" + + class CommentTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -147,42 +184,6 @@ def setUp(self) -> None: cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - def test_dismiss_requires_comment_no_js(self) -> None: - """Test that dismissing a suggestion requires a comment (no-JS case)""" - url = reverse("webview:suggestions_view") - - # Try to dismiss without a comment (non-JS behavior) - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "rejected", - "comment": "", # Empty comment - "no-js": "", # Indicate non-JS mode - }, - ) - - # Should redirect back to the same page - self.assertEqual(response.status_code, 302) - - # Follow the redirect and check for Django messages - follow_response = self.client.get(url) - self.assertEqual(follow_response.status_code, 200) - - # Check that the correct error message was added to Django messages - messages = list(get_messages(follow_response.wsgi_request)) - self.assertEqual(len(messages), 1) - self.assertEqual(str(messages[0]), "You must provide a dismissal comment") - - # Verify the suggestion is still in the suggestions view (not dismissed) - suggestions = follow_response.context["object_list"] - our_suggestion = next( - (s for s in suggestions if s.proposal_id == self.suggestion.pk), None - ) - self.assertIsNotNone( - our_suggestion, "Suggestion should still be in pending status" - ) - def test_dismiss_with_comment_succeeds(self) -> None: """Test that dismissing with a comment works and the comment appears in the view context""" From 8367c007f013e640fe5046975ff394c4946407a4 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 13:32:00 +0100 Subject: [PATCH 09/12] refactor: migate testing mandatory comments to `pytest` fixtures (3/6) --- src/webview/tests/test_comments.py | 75 ++++++++++++++++-------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/src/webview/tests/test_comments.py b/src/webview/tests/test_comments.py index f64ba3c9..707c6b2e 100644 --- a/src/webview/tests/test_comments.py +++ b/src/webview/tests/test_comments.py @@ -89,6 +89,45 @@ def test_dismiss_requires_comment_no_js( assert our_suggestion is not None, "Suggestion should still be in pending status" +def test_dismiss_with_comment_succeeds( + authenticated_client: Client, cached_suggestion: CVEDerivationClusterProposal +) -> None: + """Test that dismissing with a comment works and the comment appears in the view context""" + + url = reverse("webview:suggestions_view") + dismissal_comment = ( + "This suggestion is not relevant because the package is deprecated." + ) + + # Dismiss with a comment + response = authenticated_client.post( + url, + { + "suggestion_id": cached_suggestion.pk, + "new_status": "rejected", + "comment": dismissal_comment, + }, + ) + + # Should succeed + assert response.status_code == 200 + + # Verify the suggestion appears in dismissed view with the comment + dismissed_response = authenticated_client.get(reverse("webview:dismissed_view")) + assert dismissed_response.status_code == 200 + + # Find the suggestion in the context + suggestions = dismissed_response.context["object_list"] + our_suggestion = next( + (s for s in suggestions if s.proposal_id == cached_suggestion.pk), None + ) + assert our_suggestion is not None + + # Verify the comment appears in the suggestion context + suggestion_in_context = dismissed_response.context["object_list"][0].proposal + assert suggestion_in_context.comment == dismissal_comment + + class CommentTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -184,42 +223,6 @@ def setUp(self) -> None: cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - def test_dismiss_with_comment_succeeds(self) -> None: - """Test that dismissing with a comment works and the comment appears in the view context""" - - url = reverse("webview:suggestions_view") - dismissal_comment = ( - "This suggestion is not relevant because the package is deprecated." - ) - - # Dismiss with a comment - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "rejected", - "comment": dismissal_comment, - }, - ) - - # Should succeed - self.assertEqual(response.status_code, 200) - - # Verify the suggestion appears in dismissed view with the comment - dismissed_response = self.client.get(reverse("webview:dismissed_view")) - self.assertEqual(dismissed_response.status_code, 200) - - # Find the suggestion in the context - suggestions = dismissed_response.context["object_list"] - our_suggestion = next( - (s for s in suggestions if s.proposal_id == self.suggestion.pk), None - ) - self.assertIsNotNone(our_suggestion) - - # Verify the comment appears in the suggestion context - suggestion_in_context = dismissed_response.context["object_list"][0].proposal - self.assertEqual(suggestion_in_context.comment, dismissal_comment) - def test_accept_without_comment_succeeds(self) -> None: """Test that accepting a suggestion without a comment is allowed""" url = reverse("webview:suggestions_view") From 6f2838741c67db5ddbcededc749026d67b00ca04 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 13:35:32 +0100 Subject: [PATCH 10/12] refactor: migate testing mandatory comments to `pytest` fixtures (4/6) --- src/webview/tests/test_comments.py | 59 ++++++++++++++++-------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/webview/tests/test_comments.py b/src/webview/tests/test_comments.py index 707c6b2e..2cee2ddf 100644 --- a/src/webview/tests/test_comments.py +++ b/src/webview/tests/test_comments.py @@ -128,6 +128,37 @@ def test_dismiss_with_comment_succeeds( assert suggestion_in_context.comment == dismissal_comment +def test_accept_without_comment_succeeds( + authenticated_client: Client, cached_suggestion: CVEDerivationClusterProposal +) -> None: + """Test that accepting a suggestion without a comment is allowed""" + url = reverse("webview:suggestions_view") + + # Accept without a comment + response = authenticated_client.post( + url, + { + "suggestion_id": cached_suggestion.pk, + "new_status": "accepted", + "comment": "", # Empty comment + }, + ) + + # Should succeed + assert response.status_code == 200 + + # Verify the suggestion appears in drafts view + drafts_response = authenticated_client.get(reverse("webview:drafts_view")) + assert drafts_response.status_code == 200 + + # Find our suggestion in the context + suggestions = drafts_response.context["object_list"] + suggestion = next( + (s for s in suggestions if s.proposal_id == cached_suggestion.pk), None + ) + assert suggestion is not None + + class CommentTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -223,34 +254,6 @@ def setUp(self) -> None: cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - def test_accept_without_comment_succeeds(self) -> None: - """Test that accepting a suggestion without a comment is allowed""" - url = reverse("webview:suggestions_view") - - # Accept without a comment - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "accepted", - "comment": "", # Empty comment - }, - ) - - # Should succeed - self.assertEqual(response.status_code, 200) - - # Verify the suggestion appears in drafts view - drafts_response = self.client.get(reverse("webview:drafts_view")) - self.assertEqual(drafts_response.status_code, 200) - - # Find our suggestion in the context - suggestions = drafts_response.context["object_list"] - suggestion = next( - (s for s in suggestions if s.proposal_id == self.suggestion.pk), None - ) - self.assertIsNotNone(suggestion) - def test_accept_with_comment_shows_comment_in_context(self) -> None: """Test that accepting with a comment shows the comment in the view context""" url = reverse("webview:suggestions_view") From df3f1393f6ebba598613268b559e6f0dca658f5e Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 13:37:13 +0100 Subject: [PATCH 11/12] refactor: migate testing mandatory comments to `pytest` fixtures (5/6) --- src/webview/tests/test_comments.py | 55 ++++++++++++++++-------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/webview/tests/test_comments.py b/src/webview/tests/test_comments.py index 2cee2ddf..ef1758d2 100644 --- a/src/webview/tests/test_comments.py +++ b/src/webview/tests/test_comments.py @@ -159,6 +159,35 @@ def test_accept_without_comment_succeeds( assert suggestion is not None +def test_accept_with_comment_shows_comment_in_context( + authenticated_client: Client, cached_suggestion: CVEDerivationClusterProposal +) -> None: + """Test that accepting with a comment shows the comment in the view context""" + url = reverse("webview:suggestions_view") + acceptance_comment = "This looks good, creating draft issue." + + # Accept with a comment + response = authenticated_client.post( + url, + { + "suggestion_id": cached_suggestion.pk, + "new_status": "accepted", + "comment": acceptance_comment, + }, + ) + + # Should succeed + assert response.status_code == 200 + + # Verify the suggestion appears in drafts view with the comment + drafts_response = authenticated_client.get(reverse("webview:drafts_view")) + assert drafts_response.status_code == 200 + + # Find the suggestion in the context and verify the comment + suggestion = drafts_response.context["object_list"][0].proposal + assert suggestion.comment == acceptance_comment + + class CommentTests(TestCase): def setUp(self) -> None: # Create user and log in @@ -254,32 +283,6 @@ def setUp(self) -> None: cache_new_suggestions(self.suggestion) self.suggestion.refresh_from_db() - def test_accept_with_comment_shows_comment_in_context(self) -> None: - """Test that accepting with a comment shows the comment in the view context""" - url = reverse("webview:suggestions_view") - acceptance_comment = "This looks good, creating draft issue." - - # Accept with a comment - response = self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "accepted", - "comment": acceptance_comment, - }, - ) - - # Should succeed - self.assertEqual(response.status_code, 200) - - # Verify the suggestion appears in drafts view with the comment - drafts_response = self.client.get(reverse("webview:drafts_view")) - self.assertEqual(drafts_response.status_code, 200) - - # Find the suggestion in the context and verify the comment - suggestion = drafts_response.context["object_list"][0].proposal - self.assertEqual(suggestion.comment, acceptance_comment) - def test_updating_comment_on_existing_suggestion(self) -> None: """Test that updating a comment on an existing suggestion works""" # First accept with initial comment From d7002692a4696f52a393afcdddee9c162eb81921 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Fri, 19 Dec 2025 13:39:09 +0100 Subject: [PATCH 12/12] refactor: migate testing mandatory comments to `pytest` fixtures (6/6) This also removes the now-obsolete `TestCase` setup. --- src/webview/tests/test_comments.py | 197 ++++++----------------------- 1 file changed, 41 insertions(+), 156 deletions(-) diff --git a/src/webview/tests/test_comments.py b/src/webview/tests/test_comments.py index ef1758d2..0448b4d1 100644 --- a/src/webview/tests/test_comments.py +++ b/src/webview/tests/test_comments.py @@ -1,30 +1,8 @@ -from allauth.socialaccount.models import SocialAccount -from django.contrib.auth.models import User from django.contrib.messages import get_messages -from django.test import Client, TestCase +from django.test import Client from django.urls import reverse -from shared.listeners.cache_suggestions import cache_new_suggestions -from shared.models.cve import ( - AffectedProduct, - CveRecord, - Description, - Metric, - Organization, - Version, -) -from shared.models.linkage import ( - CVEDerivationClusterProposal, - DerivationClusterProposalLink, - ProvenanceFlags, -) -from shared.models.nix_evaluation import ( - NixChannel, - NixDerivation, - NixDerivationMeta, - NixEvaluation, - NixMaintainer, -) +from shared.models.linkage import CVEDerivationClusterProposal def test_dismiss_requires_comment_htmx( @@ -188,135 +166,42 @@ def test_accept_with_comment_shows_comment_in_context( assert suggestion.comment == acceptance_comment -class CommentTests(TestCase): - def setUp(self) -> None: - # Create user and log in - self.user = User.objects.create_user(username="admin", password="pw") - self.user.is_staff = True - self.user.save() - - # Create a GitHub social account for the user - SocialAccount.objects.get_or_create( - user=self.user, - provider="github", - uid="123456", - extra_data={"login": "admin"}, - ) - - self.client = Client() - self.client.login(username="admin", password="pw") - - # Create CVE and related objects - self.assigner = Organization.objects.create(uuid=1, short_name="foo") - self.cve_record = CveRecord.objects.create( - cve_id="CVE-2025-0001", - assigner=self.assigner, - ) - self.description = Description.objects.create(value="Test description") - self.metric = Metric.objects.create(format="cvssV3_1", raw_cvss_json={}) - self.affected_product = AffectedProduct.objects.create( - package_name="dummy-package" - ) - self.affected_product.versions.add( - Version.objects.create(status=Version.Status.AFFECTED, version="1.0") - ) - self.cve_container = self.cve_record.container.create( - provider=self.assigner, - title="Dummy Title", - ) - self.cve_container.affected.add(self.affected_product) - self.cve_container.descriptions.add(self.description) - self.cve_container.metrics.add(self.metric) - - # Create maintainer and metadata - self.maintainer = NixMaintainer.objects.create( - github_id=123, - github="testuser", - name="Test User", - email="test@example.com", - ) - self.meta = NixDerivationMeta.objects.create( - description="Dummy derivation", - insecure=False, - available=True, - broken=False, - unfree=False, - unsupported=False, - ) - self.meta.maintainers.add(self.maintainer) - - # Create evaluation and derivation - self.evaluation = NixEvaluation.objects.create( - channel=NixChannel.objects.create( - staging_branch="release-24.05", - channel_branch="nixos-24.05", - head_sha1_commit="deadbeef", - state=NixChannel.ChannelState.STABLE, - release_version="24.05", - repository="https://github.com/NixOS/nixpkgs", - ), - commit_sha1="deadbeef", - state=NixEvaluation.EvaluationState.COMPLETED, - ) - - self.derivation = NixDerivation.objects.create( - attribute="package1", - derivation_path="/nix/store/package1.drv", - name="package1-1.0", - metadata=self.meta, - system="x86_64-linux", - parent_evaluation=self.evaluation, - ) - - # Create suggestion and link derivation - self.suggestion = CVEDerivationClusterProposal.objects.create( - status=CVEDerivationClusterProposal.Status.PENDING, - cve_id=self.cve_record.pk, - ) - DerivationClusterProposalLink.objects.create( - proposal=self.suggestion, - derivation=self.derivation, - provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, - ) - - # Cache the suggestion - cache_new_suggestions(self.suggestion) - self.suggestion.refresh_from_db() - - def test_updating_comment_on_existing_suggestion(self) -> None: - """Test that updating a comment on an existing suggestion works""" - # First accept with initial comment - initial_comment = "Initial comment" - url = reverse("webview:suggestions_view") - - self.client.post( - url, - { - "suggestion_id": self.suggestion.pk, - "new_status": "accepted", - "comment": initial_comment, - }, - ) - - # Now update just the comment (no status change) - updated_comment = "Updated comment with more details" - drafts_url = reverse("webview:drafts_view") - - response = self.client.post( - drafts_url, - { - "suggestion_id": self.suggestion.pk, - "comment": updated_comment, - # No new_status means just updating comment - }, - ) - - # Should succeed - self.assertEqual(response.status_code, 200) - - # Verify the updated comment appears in the context - drafts_response = self.client.get(drafts_url) - self.assertEqual(drafts_response.status_code, 200) - - suggestion = drafts_response.context["object_list"][0].proposal - self.assertEqual(suggestion.comment, updated_comment) +def test_updating_comment_on_existing_suggestion( + authenticated_client: Client, cached_suggestion: CVEDerivationClusterProposal +) -> None: + """Test that updating a comment on an existing suggestion works""" + # First accept with initial comment + initial_comment = "Initial comment" + url = reverse("webview:suggestions_view") + + authenticated_client.post( + url, + { + "suggestion_id": cached_suggestion.pk, + "new_status": "accepted", + "comment": initial_comment, + }, + ) + + # Now update just the comment (no status change) + updated_comment = "Updated comment with more details" + drafts_url = reverse("webview:drafts_view") + + response = authenticated_client.post( + drafts_url, + { + "suggestion_id": cached_suggestion.pk, + "comment": updated_comment, + # No new_status means just updating comment + }, + ) + + # Should succeed + assert response.status_code == 200 + + # Verify the updated comment appears in the context + drafts_response = authenticated_client.get(drafts_url) + assert drafts_response.status_code == 200 + + suggestion = drafts_response.context["object_list"][0].proposal + assert suggestion.comment == updated_comment