-
-
Notifications
You must be signed in to change notification settings - Fork 7k
URLField serializer is not playing nice with "built in" test server address http://testserver #9705
-
Initially encountered in our
- Local runtime schema serialization endpoints dandi/dandi-archive#2386
where our response model containsschema_urlwhich is aURLFieldand when we plugged in a "proper" URL to local test instancehttp://testserver/api/schema/latest/dandiset/, then response we were getting from server became with code 400 and.contentsaying
(Pdb) p resp.content
b'{"schema_url":["Enter a valid URL."]}'
with such minimal reproducer depicting the issue:
from rest_framework import serializers class MySerializer(serializers.Serializer): url = serializers.URLField() def test_valid_url(): serializer = MySerializer(data={'url': 'http://testserver/path'}) serializer.is_valid()
running pytest on which would give you an ugly excepted demanding configuration since i18n would not yet be configured to give a proper localized error...
but replacing that simple hostname testserver with a domain name with the TLD (e.g. testserver.com), which is AFAIK is not required for a URL to be valid, pytest would pass just fine.
❯ sed -i -E 's,(testserver),1円.com,g' /tmp/tttt.py
❯ python -m pytest -s -v /tmp/tttt.py
====================================================================================================== test session starts =======================================================================================================
platform linux -- Python 3.11.12, pytest-8.3.5, pluggy-1.6.0 -- /home/yoh/proj/dandi/dandi-archive/.venv/bin/python
cachedir: .pytest_cache
django: version: 4.2.21
rootdir: /tmp
plugins: django-s3-file-field-1.0.1, Faker-37.3.0, mock-3.14.0, memray-1.7.0, factoryboy-2.7.0, django-4.11.1, cov-6.1.1
collected 1 item
../../../../../tmp/tttt.py::test_valid_url PASSED
So
- any way to workaround?
- if I got it right such validation stems from DJANGO itself, so should it be checked/fixed there?
Beta Was this translation helpful? Give feedback.
All reactions
- if I got it right such validation stems from DJANGO itself, so should it be checked/fixed there?
It does. I was able to reproduce the problem with a simpler test case:
from django.core.validators import URLValidator def test_url_validator(): validator = URLValidator() assert validator("http://testserver/path") is None # fails
- any way to workaround?
Change the base URL to something with a TLD? I usually have a constant in my settings to be able to switch it between test/dev/staging/prod. Django has the standard MEDIA_URL for media file, you can make up your own to fit your use case. This testserver doesn't come from DRF either, it's vanilla Django too.
Not much specific t...
Replies: 2 comments 3 replies
-
you should have open a discussion first
Beta Was this translation helpful? Give feedback.
All reactions
-
Converted to discussion
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
thanks for the quick handling! whatever makes us unstuck faster is great.
Beta Was this translation helpful? Give feedback.
All reactions
-
- if I got it right such validation stems from DJANGO itself, so should it be checked/fixed there?
It does. I was able to reproduce the problem with a simpler test case:
from django.core.validators import URLValidator def test_url_validator(): validator = URLValidator() assert validator("http://testserver/path") is None # fails
- any way to workaround?
Change the base URL to something with a TLD? I usually have a constant in my settings to be able to switch it between test/dev/staging/prod. Django has the standard MEDIA_URL for media file, you can make up your own to fit your use case. This testserver doesn't come from DRF either, it's vanilla Django too.
Not much specific to DRF, basically, it's all Django.
Beta Was this translation helpful? Give feedback.
All reactions
-
Thanks! Indeed works if we provide custom name in APIClient(SERVER_NAME=TEST_SERVER_NAME) invocation, which also required adjusting ALLOWED_HOSTS.
Here is a complete patch which I worked out
commit 3a348e80f0fb2c46da90556884fe688cbf618db9 Author: Yaroslav O. Halchenko <debian@onerussian.com> Date: Thu May 22 09:52:33 2025 -0400 Provide custom name for django testserver instance The default name django uses is "testserver" and then URL becomes http://testserver, but DJANGO itself does not consider it to be a valid URL because it lacks TLD. So whenever we add "schema_url" pointing to our server (in tests just "testserver") DJANGO starts raising validation errors. See https://github.com/encode/django-rest-framework/discussions/9705 for more information/rationale. diff --git a/dandiapi/api/tests/test_info.py b/dandiapi/api/tests/test_info.py index 3ab461ba..22e849ec 100644 --- a/dandiapi/api/tests/test_info.py +++ b/dandiapi/api/tests/test_info.py @@ -4,6 +4,7 @@ from django.conf import settings from django.urls import reverse from dandiapi import __version__ +from dandiapi.conftest import TEST_SERVER_NAME def test_rest_info(api_client): @@ -11,7 +12,7 @@ def test_rest_info(api_client): assert resp.status_code == 200 # Get the expected schema URL - schema_url = f'http://testserver{reverse("schema-dandiset-latest")}' + schema_url = f'http://{TEST_SERVER_NAME}{reverse("schema-dandiset-latest")}' assert resp.json() == { 'schema_version': settings.DANDI_SCHEMA_VERSION, diff --git a/dandiapi/conftest.py b/dandiapi/conftest.py index 332b8640..74a79bbe 100644 --- a/dandiapi/conftest.py +++ b/dandiapi/conftest.py @@ -49,6 +49,9 @@ register(UploadFactory) register(ZarrArchiveFactory) register(EmbargoedZarrArchiveFactory, _name='embargoed_zarr_archive') +# Should have TLD since otherwise would invalidate the URL +TEST_SERVER_NAME = 'testserver.dandiarchive.org' + # Register zarr file/directory factories @pytest.fixture @@ -77,14 +80,21 @@ def version(request): return request.param() +def _api_client() -> APIClient: + """Get a new client for a fixture.""" + if TEST_SERVER_NAME not in settings.ALLOWED_HOSTS: + settings.ALLOWED_HOSTS.append(TEST_SERVER_NAME) + return APIClient(SERVER_NAME=TEST_SERVER_NAME) + + @pytest.fixture def api_client() -> APIClient: - return APIClient() + return _api_client() @pytest.fixture def authenticated_api_client(user) -> APIClient: - client = APIClient() + client = _api_client() client.force_authenticate(user=user) return client
Beta Was this translation helpful? Give feedback.