Hello, please find the following patches. One fixes a bug I noticed where the filter URLs generated by the [mirrorlist] form were not filtering as they should. While fixing this bug, I also improved (I hope!) the testing in the area. Let me know if I'm missing anything.
I ran all the test cases, and they still pass. On that note, thanks for making the testing setup very easy to use. It is great to see.
[mirrorlist]: https://www.archlinux.org/mirrorlist/
Genki Sky (2): mirrorlist: Accept GET parameters as filters mirrorlist: Complete /all/https success test case
mirrors/tests/__init__.py | 11 ++++++----- mirrors/tests/test_mirrorlist.py | 30 +++++++++++++++++++++++------- mirrors/views/mirrorlist.py | 3 ++- 3 files changed, 31 insertions(+), 13 deletions(-)
-- 2.17.0
This fixes a regression. Originally request.REQUEST was used, but django 1.9 removed this. In its stead, request.POST was used unconditionally. However, this results in any GET request returning *all* mirrors, rather than filtering as requested in the parameters.
This patch uses POST or GET based on the request method. This fixes the behavior of the [mirror-filter-form], and any scripts depending on the generated URL format. Accordingly, make test_mirrorlist_filter() test both the success and failure cases, rather than just success.
[mirror-filter-form]: https://www.archlinux.org/mirrorlist/
Signed-off-by: Genki Sky sky@genki.is --- mirrors/tests/__init__.py | 11 ++++++----- mirrors/tests/test_mirrorlist.py | 16 ++++++++++++++-- mirrors/views/mirrorlist.py | 3 ++- 3 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/mirrors/tests/__init__.py b/mirrors/tests/__init__.py index fb6c10d..a1d3c2c 100644 --- a/mirrors/tests/__init__.py +++ b/mirrors/tests/__init__.py @@ -1,12 +1,13 @@ from mirrors.models import MirrorUrl, MirrorProtocol, Mirror
-def create_mirror_url(): - mirror = Mirror.objects.create(name='mirror1', +def create_mirror_url(name='mirror1', country='US', + protocol='http', url='https://archlinux.org'): + mirror = Mirror.objects.create(name=name, admin_email='admin@archlinux.org') - mirror_protocol = MirrorProtocol.objects.create(protocol='http') - mirror_url = MirrorUrl.objects.create(url='https://archlinux.org', + mirror_protocol = MirrorProtocol.objects.create(protocol=protocol) + mirror_url = MirrorUrl.objects.create(url=url, protocol=mirror_protocol, mirror=mirror, - country='US') + country=country) return mirror_url diff --git a/mirrors/tests/test_mirrorlist.py b/mirrors/tests/test_mirrorlist.py index 5590a96..1ad3d8d 100644 --- a/mirrors/tests/test_mirrorlist.py +++ b/mirrors/tests/test_mirrorlist.py @@ -30,9 +30,21 @@ def test_mirrorlist_all_https(self): # TODO: test 200 case
def test_mirrorlist_filter(self): - response = self.client.get('/mirrorlist/?country=all&protocol=http&ip_version=4') + jp_mirror_url = create_mirror_url( + name='jp_mirror', + country='JP', + protocol='https', + url='https://wikipedia.jp') + + # First test that we correctly see the above mirror. + response = self.client.get('/mirrorlist/?country=JP&protocol=https') self.assertEqual(response.status_code, 200) - self.assertIn(self.mirror_url.hostname, response.content) + self.assertIn(jp_mirror_url.hostname, response.content) + + # Now confirm that the US mirror did not show up. + self.assertNotIn(self.mirror_url.hostname, response.content) + + jp_mirror_url.delete()
def test_generate(self): response = self.client.get('/mirrorlist/?country=all&protocol=http&ip_version=4') diff --git a/mirrors/views/mirrorlist.py b/mirrors/views/mirrorlist.py index 35d59e8..45c0181 100644 --- a/mirrors/views/mirrorlist.py +++ b/mirrors/views/mirrorlist.py @@ -55,7 +55,8 @@ def as_div(self): @csrf_exempt def generate_mirrorlist(request): if request.method == 'POST' or len(request.GET) > 0: - form = MirrorlistForm(data=request.POST) + data = request.POST if request.method == 'POST' else request.GET + form = MirrorlistForm(data=data) if form.is_valid(): countries = form.cleaned_data['country'] protocols = form.cleaned_data['protocol']
Also, remove test_generate(), as it was testing no more than test_mirrorlist_filter() already was.
Signed-off-by: Genki Sky sky@genki.is --- mirrors/tests/test_mirrorlist.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/mirrors/tests/test_mirrorlist.py b/mirrors/tests/test_mirrorlist.py index 1ad3d8d..9e20812 100644 --- a/mirrors/tests/test_mirrorlist.py +++ b/mirrors/tests/test_mirrorlist.py @@ -25,9 +25,18 @@ def test_mirrorlist_all_http(self): self.assertIn(self.mirror_url.hostname, response.content)
def test_mirrorlist_all_https(self): + # First test that without any https mirrors, we get a 404. response = self.client.get('/mirrorlist/all/https/') self.assertEqual(response.status_code, 404) - # TODO: test 200 case + + # Now, after adding an HTTPS mirror, we expect to succeed. + https_mirror_url = create_mirror_url( + name='https_mirror', + protocol='https', + url='https://wikipedia.org') + response = self.client.get('/mirrorlist/all/https/') + self.assertEqual(response.status_code, 200) + https_mirror_url.delete()
def test_mirrorlist_filter(self): jp_mirror_url = create_mirror_url( @@ -45,8 +54,3 @@ def test_mirrorlist_filter(self): self.assertNotIn(self.mirror_url.hostname, response.content)
jp_mirror_url.delete() - - def test_generate(self): - response = self.client.get('/mirrorlist/?country=all&protocol=http&ip_version=4') - self.assertEqual(response.status_code, 200) - self.assertIn(self.mirror_url.hostname, response.content)
arch-projects@lists.archlinux.org