From 08d76ba9701888b0333b340faf79bea47312c1c0 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Mon, 25 Nov 2024 15:34:54 +0100 Subject: [PATCH 1/3] Add unit tests that should pass --- .../issues/tests/test_issue.py | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/backend/code_review_backend/issues/tests/test_issue.py b/backend/code_review_backend/issues/tests/test_issue.py index 070ca4679..4d06092f8 100644 --- a/backend/code_review_backend/issues/tests/test_issue.py +++ b/backend/code_review_backend/issues/tests/test_issue.py @@ -5,6 +5,7 @@ from datetime import datetime from unittest.mock import patch +from django.db.utils import IntegrityError from django.test import TestCase from django.urls import reverse from rest_framework import status @@ -211,3 +212,63 @@ def test_list_repository_issues_date_no_match(self): data = response.json() self.assertEqual(data["count"], 0) self.assertEqual(data["results"], []) + + def test_issue_link_unicity_all_values(self): + """ + Check that the unique constraints are respected when all positioning values are set + """ + self.revision.issue_links.create( + issue=self.err_issue, line=20, nb_lines=2, char=1 + ) + with self.assertRaises(IntegrityError): + self.revision.issue_links.create( + issue=self.err_issue, line=20, nb_lines=2, char=1 + ) + + def test_issue_link_unicity_no_line(self): + """ + Check that the unique constraints are respected when line is not set + """ + self.revision.issue_links.create( + issue=self.err_issue, line=None, nb_lines=2, char=1 + ) + with self.assertRaises(IntegrityError): + self.revision.issue_links.create( + issue=self.err_issue, line=None, nb_lines=2, char=1 + ) + + def test_issue_link_unicity_no_nb_lines(self): + """ + Check that the unique constraints are respected when nb_lines is not set + """ + self.revision.issue_links.create( + issue=self.err_issue, line=100, nb_lines=None, char=1 + ) + with self.assertRaises(IntegrityError): + self.revision.issue_links.create( + issue=self.err_issue, line=100, nb_lines=None, char=1 + ) + + def test_issue_link_unicity_no_char(self): + """ + Check that the unique constraints are respected when char is not set + """ + self.revision.issue_links.create( + issue=self.err_issue, line=100, nb_lines=42, char=None + ) + with self.assertRaises(IntegrityError): + self.revision.issue_links.create( + issue=self.err_issue, line=100, nb_lines=42, char=None + ) + + def test_issue_link_unicity_no_position(self): + """ + Check that the unique constraints are respected when no positioning args are set + """ + self.revision.issue_links.create( + issue=self.err_issue, line=None, nb_lines=None, char=None + ) + with self.assertRaises(IntegrityError): + self.revision.issue_links.create( + issue=self.err_issue, line=None, nb_lines=None, char=None + ) From c01f6085f2042d16370501e014bd0ee2ca584717 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Mon, 25 Nov 2024 16:05:07 +0100 Subject: [PATCH 2/3] Add constraint for Postgresql 15 --- .../0015_unique_issue_link_positions.py | 28 +++++++++++++++++++ backend/code_review_backend/issues/models.py | 10 ++----- 2 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 backend/code_review_backend/issues/migrations/0015_unique_issue_link_positions.py diff --git a/backend/code_review_backend/issues/migrations/0015_unique_issue_link_positions.py b/backend/code_review_backend/issues/migrations/0015_unique_issue_link_positions.py new file mode 100644 index 000000000..c411965d5 --- /dev/null +++ b/backend/code_review_backend/issues/migrations/0015_unique_issue_link_positions.py @@ -0,0 +1,28 @@ +# Generated by Django 5.1.1 on 2024-11-25 14:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("issues", "0014_unique_hash"), + ] + + operations = [ + migrations.RemoveConstraint( + model_name="issuelink", + name="issue_link_unique_revision", + ), + migrations.RemoveConstraint( + model_name="issuelink", + name="issue_link_unique_diff", + ), + migrations.AddConstraint( + model_name="issuelink", + constraint=models.UniqueConstraint( + fields=("issue", "revision", "diff", "line", "nb_lines", "char"), + name="issue_link_unique", + nulls_distinct=False, + ), + ), + ] diff --git a/backend/code_review_backend/issues/models.py b/backend/code_review_backend/issues/models.py index 38ed6a264..78d28b86c 100644 --- a/backend/code_review_backend/issues/models.py +++ b/backend/code_review_backend/issues/models.py @@ -171,16 +171,10 @@ class IssueLink(models.Model): class Meta: constraints = [ - # Two constraints are required as Null values are not compared for unicity - models.UniqueConstraint( - fields=["issue", "revision", "line", "nb_lines", "char"], - name="issue_link_unique_revision", - condition=Q(diff__isnull=True), - ), models.UniqueConstraint( fields=["issue", "revision", "diff", "line", "nb_lines", "char"], - name="issue_link_unique_diff", - condition=Q(diff__isnull=False), + name="issue_link_unique", + nulls_distinct=False, ), ] From 3916a22fe0df4d64dafae48f14929ae9e1f5e520 Mon Sep 17 00:00:00 2001 From: Bastien Abadie Date: Mon, 25 Nov 2024 16:31:38 +0100 Subject: [PATCH 3/3] Run backend unit tests with postgresql --- .taskcluster.yml | 3 +++ backend/ci/pg_hba.conf | 5 +++++ backend/ci/setup_postgres.sh | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+) create mode 100644 backend/ci/pg_hba.conf create mode 100755 backend/ci/setup_postgres.sh diff --git a/.taskcluster.yml b/.taskcluster.yml index 9fd52a60b..5bb542a82 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -202,7 +202,10 @@ tasks: - "git clone --quiet ${repository} /src && cd /src && git checkout ${head_rev} -b checks && cd /src/tools && ${pip_install} . && cd /src/backend && ${pip_install} . && ${pip_install} -r requirements-dev.txt && + ./ci/setup_postgres.sh && ./manage.py test && ./manage.py makemigrations --check --noinput --dry-run -v 3" + env: + DATABASE_URL: postgres://tester@127.0.0.1/code-review metadata: name: "Code Review Backend checks: unit tests" description: Check python code with Django tests diff --git a/backend/ci/pg_hba.conf b/backend/ci/pg_hba.conf new file mode 100644 index 000000000..6a916e8d8 --- /dev/null +++ b/backend/ci/pg_hba.conf @@ -0,0 +1,5 @@ +# Database administrative login by Unix domain socket +local all postgres peer + +# Only tester user can connect locally, without password +host all tester 127.0.0.1/32 trust diff --git a/backend/ci/setup_postgres.sh b/backend/ci/setup_postgres.sh new file mode 100755 index 000000000..4e7a36942 --- /dev/null +++ b/backend/ci/setup_postgres.sh @@ -0,0 +1,19 @@ +#!/bin/bash -ex +export PG_VERSION=15 + +# Silent apt +export DEBIAN_FRONTEND=noninteractive + +# Install postgresql +apt update +apt install -qq -y postgresql-$PG_VERSION + +# Setup access rights +cp $(dirname $0)/pg_hba.conf /etc/postgresql/$PG_VERSION/main/pg_hba.conf + +# Start postgresql +pg_ctlcluster $PG_VERSION main start + +# Create user & database +su postgres -c 'createuser --createdb tester' +su postgres -c 'createdb --owner=tester code-review'