Skip to content

Commit

Permalink
Fix a CI issue that causes unit tests to run against the wrong versio…
Browse files Browse the repository at this point in the history
…n of CVAT (#5612)

There seems to be a bug somewhere in the Docker ecosystem (it's probably
either Docker Compose, Docker Buildx or BuildKit) that causes `docker
compose build` to ignore base images that are already present in the
system, and instead fetch them from Docker Hub, if there's a custom
Buildx builder configured. There's a bug report here:
<docker/compose#9939>.

This bug means that when the build pipeline builds the `cvat_ci` image,
it's based on the latest release of `cvat/server` from Docker Hub
instead of the version that we just built. Consequently, we run the unit
tests against that release instead of the development version.

Fortunately, we don't actually need to set up a Buildx builder in most
jobs (including the `unit_testing` job), so just don't do that.

Also, use `cvat/server:local` as the base image in `Dockerfile.ci`. This
will prevent a similar bug from reoccurring in the future, since the
`local` tag should never be uploaded to Docker Hub.
  • Loading branch information
SpecLad authored and sizov-kirill committed Jan 30, 2023
1 parent c9c7f55 commit 22f4b65
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
9 changes: 0 additions & 9 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,6 @@ jobs:
with:
python-version: '3.8'

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2

- name: Download CVAT server image
uses: actions/download-artifact@v3
with:
Expand Down Expand Up @@ -195,9 +192,6 @@ jobs:
steps:
- uses: actions/checkout@v3

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2

- name: Download CVAT server image
uses: actions/download-artifact@v3
with:
Expand Down Expand Up @@ -265,9 +259,6 @@ jobs:
steps:
- uses: actions/checkout@v3

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2

- uses: actions/setup-node@v3
with:
node-version: '16.x'
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile.ci
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM cvat/server
FROM cvat/server:local

ENV DJANGO_CONFIGURATION=testing
USER root
Expand Down
9 changes: 8 additions & 1 deletion cvat/apps/engine/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from cvat.apps.engine.log import slogger
from cvat.apps.engine.utils import parse_specific_attributes

from drf_spectacular.utils import OpenApiExample, extend_schema_serializer
from drf_spectacular.utils import OpenApiExample, extend_schema_field, extend_schema_serializer

class BasicUserSerializer(serializers.ModelSerializer):
def validate(self, attrs):
Expand Down Expand Up @@ -926,6 +926,13 @@ class FrameMetaSerializer(serializers.Serializer):
name = serializers.CharField(max_length=1024)
related_files = serializers.IntegerField()

# for compatibility with version 2.3.0
has_related_context = serializers.SerializerMethodField()

@extend_schema_field(serializers.BooleanField)
def get_has_related_context(self, obj: dict) -> bool:
return obj['related_files'] != 0

class PluginsSerializer(serializers.Serializer):
GIT_INTEGRATION = serializers.BooleanField()
ANALYTICS = serializers.BooleanField()
Expand Down
13 changes: 10 additions & 3 deletions cvat/apps/iam/tests/test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def test_can_report_denial_reason(self):
ForceLogin(user=self.user, client=self.client):
response = self.client.get(self.ENDPOINT_WITH_AUTH)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json(), expected_reasons)
self.assertTrue(all([reason in str(response.content) for reason in expected_reasons]))

def test_can_report_merged_denial_reasons(self):
expected_reasons = [["hello", "world"], ["hi", "there"]]
Expand All @@ -159,7 +159,14 @@ def test_can_report_merged_denial_reasons(self):
ForceLogin(user=self.user, client=self.client):
response = self.client.get(self.ENDPOINT_WITH_AUTH)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json(), list(itertools.chain.from_iterable(expected_reasons)))
self.assertTrue(
all(
[
reason in str(response.content)
for reason in itertools.chain(*expected_reasons)
]
)
)

def test_can_allow_if_no_permission_matches(self):
with self._mock_permissions(), ForceLogin(user=self.user, client=self.client):
Expand All @@ -179,4 +186,4 @@ def test_can_deny_if_some_permissions_deny(self):
ForceLogin(user=self.user, client=self.client):
response = self.client.get(self.ENDPOINT_WITH_AUTH)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.json(), expected_reasons)
self.assertTrue(all([reason in str(response.content) for reason in expected_reasons]))

0 comments on commit 22f4b65

Please sign in to comment.