(no-ticket) Fix bugs not caught due to never executed tests

Previously, @needs_linux decorator was introduced to limit execution of
sinol makefile related tests to Linux only. It had a side effect of
skipping execution of those tests entirely, even on Linux.

Tests in question had to be fixed because no one noticed them for
so long. A small number of bugs was discovered throught this effort:

 - addproblem management command not starting since Django 1.10 upgrade
   due to argument parsing failure,
 - assertStreamingEqual sometimes has to let PDFs pass, therefore
   decoding what passes through it is unwise - this behaviour was
   introduced during py3 test fixing festival and is reverted here,
 - problem_site was remodeled and some buttons were moved to the new
   settings tab - tests had to be adapted.

Some tests are still failing – they are now marked xfail in strict mode,
so that anyone who fixes the underlying code will have to remove the
mark.

Change-Id: Ib9c3f681a405c8707857ef0b0f574685952f5343

oioioi/base/tests/__init__.py

@@ -181,7 +181,7 @@ def assertNoneIn(self, elems, container, msg=None):
 
 
 def needs_linux(fn):
-    return pytest.mark.skip(sys.platform not in ('linux', 'linux2'),
+    return pytest.mark.skipif(sys.platform not in ('linux', 'linux2'),
             reason="This test needs Linux")(fn)
 
 

oioioi/problems/management/commands/addproblem.py

@@ -17,8 +17,8 @@ def add_arguments(self, parser):
         parser.add_argument('no_throw',
                             type=str,
                             nargs='?',
-                            default="",
-                            choices=['nothrow'])
+                            default='',
+                            choices=['', 'nothrow'])
 
     @transaction.atomic
     def handle(self, *args, **options):

oioioi/problems/tests.py

@@ -14,6 +14,7 @@
 from django.test.utils import override_settings
 from django.utils.html import strip_tags
 from django.utils.timezone import utc
+import pytest
 import six.moves.urllib.parse
 from six.moves import range
 
@@ -450,7 +451,8 @@ def test_problem_submission_limit_changed(self):
         url = response.redirect_chain[-1][0]
         self.assertEqual(response.status_code, 200)
         response = self.client.post(url,
-                {'package_file': package_file}, follow=True)
+                {'package_file': package_file,
+                'visibility': Problem.VISIBILITY_FRIENDS}, follow=True)
         self.assertEqual(response.status_code, 200)
 
         pis = ProblemInstance.objects.filter(problem=problem)
@@ -824,6 +826,7 @@ def check_models_for_simple_package(self, problem_instance):
         for test in to_find:
             self.assertContains(response, ">" + test + "</th>")
 
+    @pytest.mark.xfail(strict=True)
     def test_upload_problem(self):
         filename = get_test_filename('test_simple_package.zip')
         self.assertTrue(self.client.login(username='test_admin'))
@@ -845,7 +848,8 @@ def test_upload_problem(self):
         self.assertIn('problems/problemset/add-or-update.html',
                 [getattr(t, 'name', None) for t in response.templates])
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(Problem.objects.count(), 1)
         self.assertEqual(ProblemInstance.objects.count(), 1)
@@ -865,24 +869,24 @@ def test_upload_problem(self):
         self.assertContains(response, "<td>tst</td>")
         # and we are problem's author and problem_site exists
         problem = Problem.objects.get()
-        url = reverse('problem_site', args=[problem.problemsite.url_key])
+        url = reverse('problem_site', args=[problem.problemsite.url_key]) + '?key=settings'
         response = self.client.post(url, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, 'Edit problem')
         self.assertContains(response, 'Reupload problem')
-        self.assertContains(response, 'Show model solutions')
+        self.assertContains(response, 'Model solutions')
         # we can see model solutions of main_problem_instance
         self.check_models_for_simple_package(problem.main_problem_instance)
 
-        # reuploading problem in problemset is not aviable from problemset
+        # reuploading problem in problemset is not available from problemset
         url = reverse('problemset_add_or_update')
         response = self.client.get(url, {'key': "problemset_source",
                                          'problem': problem.id}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "Option not available")
-        self.assertContains(response, "Update problem")
         self.assertNotContains(response, "Select")
 
+    @pytest.mark.xfail(strict=True)
     def test_add_problem_to_contest(self):
         ProblemInstance.objects.all().delete()
 
@@ -897,7 +901,8 @@ def test_add_problem_to_contest(self):
         url = response.redirect_chain[-1][0]
         self.assertEqual(response.status_code, 200)
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(Problem.objects.count(), 1)
         self.assertEqual(ProblemInstance.objects.count(), 1)
@@ -992,7 +997,8 @@ def test_add_problem_to_contest(self):
         url = response.redirect_chain[-1][0]
         self.assertEqual(response.status_code, 200)
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(ProblemInstance.objects.count(), pi_number + 1)
         self.assertContains(response, "3 PROBLEMS NEED REJUDGING")
@@ -1022,14 +1028,16 @@ def test_uploading_to_contest(self):
         self.assertIn('problems/add-or-update.html',
                 [getattr(t, 'name', None) for t in response.templates])
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(Problem.objects.count(), 1)
         self.assertEqual(ProblemInstance.objects.count(), 2)
 
         # many times
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(Problem.objects.count(), 2)
         self.assertEqual(ProblemInstance.objects.count(), 4)

oioioi/sinolpack/tests.py

@@ -194,6 +194,7 @@ def test_assign_time_limits_for_groups_from_file(self):
         self.assertEqual(tests.get(name='1c').time_limit, 2000)
         self.assertEqual(tests.get(name='2').time_limit, 3000)
 
+    @pytest.mark.xfail(strict=True)
     def test_assign_time_limits_for_groups_nonexistent(self):
         filename = get_test_filename(
             'test_time_limits_for_nonexisting_group.zip')
@@ -218,6 +219,7 @@ def test_assign_time_limits_for_different_levels(self):
         self.assertEqual(tests.get(name='1c').time_limit, 5000)
         self.assertEqual(tests.get(name='2').time_limit, 7000)
 
+    @pytest.mark.xfail(strict=True)
     def test_assign_points_nonexistent(self):
         filename = get_test_filename('test_scores_nonexistent_fail.zip')
         self.assertRaises(CommandError, call_command, 'addproblem', filename)
@@ -228,6 +230,7 @@ def test_assign_points_nonexistent(self):
         # Check if error message is relevant to the issue
         self.assertIn("no such test group exists", package.info)
 
+    @pytest.mark.xfail(strict=True)
     def test_assign_points_not_exhaustive(self):
         filename = get_test_filename('test_scores_notexhaustive_fail.zip')
         self.assertRaises(CommandError, call_command, 'addproblem', filename)
@@ -255,7 +258,8 @@ def test_huge_unpack_update(self):
         self.assertEqual(response.status_code, 200)
         url = response.redirect_chain[-1][0]
         response = self.client.post(url,
-            {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         url = reverse('oioioiadmin:problems_problempackage_changelist')
         self.assertRedirects(response, url)
@@ -457,6 +461,53 @@ def test_interactive_task(self):
         problem = Problem.objects.get()
         self._check_interactive_package(problem)
 
+    def _add_problem_with_author(self, filename, author, nothrow=False):
+        try:
+            backend = \
+                    import_string(backend_for_package(filename))()
+        except NoBackend:
+            raise ValueError("Package format not recognized")
+
+        pp = ProblemPackage(problem=None)
+        pp.package_file.save(filename, File(open(filename, 'rb')))
+        env = {'author': author}
+        pp.problem_name = backend.get_short_name(filename)
+        pp.save()
+
+        env['package_id'] = pp.id
+        problem = None
+        with pp.save_operation_status():
+            backend.unpack(env)
+            problem = Problem.objects.get(id=env['problem_id'])
+            pp.problem = problem
+            pp.save()
+
+        if problem is None and not nothrow:
+            raise ValueError("Error during unpacking the given package")
+
+    def test_restrict_html(self):
+        self.assertTrue(self.client.login(username='test_user'))
+        filename = \
+            get_test_filename('test_simple_package_with_malicious_html_statement.zip')
+
+        with self.settings(USE_SINOLPACK_MAKEFILES=False):
+            with self.settings(SINOLPACK_RESTRICT_HTML=True):
+                self._add_problem_with_author(filename, 'test_user', True)
+                self.assertEqual(Problem.objects.count(), 0)
+                package = ProblemPackage.objects.get()
+                self.assertEqual(package.status, "ERR")
+                  # Check if error message is relevant to the issue
+                self.assertIn("problem statement in HTML", package.info)
+
+                self._add_problem_with_author(filename, 'test_admin')
+
+        self._add_problem_with_author(filename, 'test_user')
+
+        with self.settings(SINOLPACK_RESTRICT_HTML=True):
+            self._add_problem_with_author(filename, 'test_user')
+
+        self.assertEqual(Problem.objects.count(), 3)
+
 
 @enable_both_unpack_configurations
 @needs_linux
@@ -481,7 +532,8 @@ def test_upload_and_download_package(self):
         self.assertIn('problems/add-or-update.html',
                 [getattr(t, 'name', None) for t in response.templates])
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(Problem.objects.count(), 1)
         self.assertEqual(ProblemInstance.objects.count(), 2)
@@ -517,7 +569,8 @@ def test_upload_and_download_package(self):
         self.assertIn('problems/add-or-update.html',
                 [getattr(t, 'name', None) for t in response.templates])
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         problem_instance = ProblemInstance.objects \
             .filter(contest__isnull=False).get()
@@ -532,6 +585,7 @@ def test_upload_and_download_package(self):
         self.assertStreamingEqual(response, open(filename, 'rb').read())
 
     @both_configurations
+    @pytest.mark.xfail(strict=True)
     def test_inwer_failure_package(self):
         ProblemInstance.objects.all().delete()
 
@@ -544,7 +598,8 @@ def test_inwer_failure_package(self):
         url = response.redirect_chain[-1][0]
         self.assertEqual(response.status_code, 200)
         response = self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(Problem.objects.count(), 0)
         self.assertEqual(ProblemInstance.objects.count(), 0)
@@ -647,71 +702,22 @@ def upload_package(self):
         self.assertIn('problems/add-or-update.html',
                 [getattr(t, 'name', None) for t in response.templates])
         return self.client.post(url,
-                {'package_file': open(filename, 'rb')}, follow=True)
+                {'package_file': open(filename, 'rb'),
+                 'visibility': Problem.VISIBILITY_PRIVATE}, follow=True)
 
+    @pytest.mark.xfail(strict=True)
     @override_settings(MAX_TEST_TIME_LIMIT_PER_PROBLEM=2000)
     def test_time_limit(self):
         response = self.upload_package()
         self.assertContains(response,
                         "Sum of time limits for all tests is too big. It's "
                         "50s, but it shouldn't exceed 2s.")
 
+    @pytest.mark.xfail(strict=True)
     @override_settings(MAX_MEMORY_LIMIT_FOR_TEST=10)
     def test_memory_limit(self):
         response = self.upload_package()
         self.assertContains(response,
                         "Memory limit mustn't be greater than %dKiB"
                         % settings.MAX_MEMORY_LIMIT_FOR_TEST)
 
-
-# TODO: When @needs_linux is fixed move tests from here to TestSinolPackage
-# Related change: https://gerrit.sio2project.mimuw.edu.pl/#/c/3161
-class TestSinolPackUnpack(TestCase):
-    fixtures = ['test_users', 'test_contest']
-
-    def _add_problem_with_author(self, filename, author, nothrow=False):
-        try:
-            backend = \
-                    import_string(backend_for_package(filename))()
-        except NoBackend:
-            raise ValueError("Package format not recognized")
-
-        pp = ProblemPackage(problem=None)
-        pp.package_file.save(filename, File(open(filename, 'rb')))
-        env = {'author': author}
-        pp.problem_name = backend.get_short_name(filename)
-        pp.save()
-
-        env['package_id'] = pp.id
-        problem = None
-        with pp.save_operation_status():
-            backend.unpack(env)
-            problem = Problem.objects.get(id=env['problem_id'])
-            pp.problem = problem
-            pp.save()
-
-        if problem is None and not nothrow:
-            raise ValueError("Error during unpacking the given package")
-
-    def test_restrict_html(self):
-        self.assertTrue(self.client.login(username='test_user'))
-        filename = \
-            get_test_filename('test_simple_package_with_malicious_html_statement.zip')
-
-        with self.settings(USE_SINOLPACK_MAKEFILES=False):
-            with self.settings(SINOLPACK_RESTRICT_HTML=True):
-                self._add_problem_with_author(filename, 'test_user', True)
-                self.assertEqual(Problem.objects.count(), 0)
-                package = ProblemPackage.objects.get()
-                self.assertEqual(package.status, "ERR")
-                  # Check if error message is relevant to the issue
-                self.assertIn("problem statement in HTML", package.info)
-
-                self._add_problem_with_author(filename, 'test_admin')
-
-        self._add_problem_with_author(filename, 'test_user')
-
-        with self.settings(SINOLPACK_RESTRICT_HTML=True):
-            self._add_problem_with_author(filename, 'test_user')
-
-        self.assertEqual(Problem.objects.count(), 3)