Repository: kamilkijak/django-smoke-tests (language: python)

Issue:
SKIP_SMOKE_TESTS ignored
This may be more a request for documentation, but it's unclear what the format of `SKIP_SMOKE_TESTS` should be.

How do you specify a URL to ignore? The README implies it should be the same name that's accepted by `reverse()`, but this doesn't seem to be true.

For example, `smoke_tests` currently complains about `/admin/myapp/mymodel/add/` with the notorious error:

     django.core.exceptions.ValidationError: ['ManagementForm data is missing or has been tampered with']

The traceback returned touches none of my own code, so there's no way for me to fix this, so I'd like `smoke_tests` to skip this path.

You'd think adding `admin:myapp_mymodel_add` to the `SKIP_SMOKE_TESTS` list would skip this path, but it has no effect. I've tried all of these patterns:

    'admin:myapp_mymodel_add'
    '^admin/myapp/mymodel/add/'
    '/admin/myapp/mymodel/add/'
    '/admin/myapp/mymodel/add'

and none of them skip that path.


Discussion / hints:

Fix patch:
diff --git a/README.rst b/README.rst
index 775ab20..47065a0 100644
--- a/README.rst
+++ b/README.rst
@@ -101,12 +101,13 @@ Skipping tests
 ~~~~~~~~~~~~~~
 To skip tests for specific URLs add ``SKIP_SMOKE_TESTS`` option in your settings.
 
-This setting should contain list of URLs' names.
+This setting should contain list of URL pattern names.
 
 .. code-block:: python
 
     SKIP_SMOKE_TESTS = (
         'all-astronauts',  # to skip url(r'^astronauts/', AllAstronauts.as_view(), name='all-astronauts')
+        'missions:all-launches',  # to skip 'all-launches' from 'missions' app
     )
 
 


Test patch:
diff --git a/django_smoke_tests/generator.py b/django_smoke_tests/generator.py
index 4b41591..51f5f84 100644
--- a/django_smoke_tests/generator.py
+++ b/django_smoke_tests/generator.py
@@ -50,7 +50,7 @@ def __init__(
         self.warnings = []
 
         # TODO: consider simplifying the structure below or introducing type hints
-        self.all_patterns = []  # [(url_pattern, lookup_str, url_name),]
+        self.all_patterns = []  # [(url_pattern, lookup_str, url_name, url_namespace, app_name),]
 
     def validate_custom_http_methods(self, http_methods):
         unsupported_methods = set(http_methods) - set(self.SUPPORTED_HTTP_METHODS)
@@ -91,9 +91,9 @@ def test(self_of_test):
 
     def execute(self):
         self.load_all_endpoints(URLResolver(r'^/', settings.ROOT_URLCONF).url_patterns)
-        for url_pattern, lookup_str, url_name in self.all_patterns:
+        for url_pattern, lookup_str, url_name, url_namespace, app_name in self.all_patterns:
             if not self.app_names or self.is_url_inside_specified_app(lookup_str):
-                self.create_tests_for_endpoint(url_pattern, url_name)
+                self.create_tests_for_endpoint(url_pattern, url_name, url_namespace, app_name)
 
         if self.disable_migrations:
             self._disable_native_migrations()
@@ -131,25 +131,30 @@ def is_url_inside_specified_app(self, lookup_str):
                 return True
         return False
 
-    def load_all_endpoints(self, url_list, parent_url=''):
+    def load_all_endpoints(self, url_list, parent_url=None, parent_namespace=None, app_name=None):
         for url_pattern in url_list:
             if hasattr(url_pattern, 'url_patterns'):
                 self.load_all_endpoints(
-                    url_pattern.url_patterns, parent_url + get_pattern(url_pattern)
+                    url_pattern.url_patterns,
+                    parent_url + get_pattern(url_pattern) if parent_url else get_pattern(url_pattern),
+                    ':'.join(filter(None, [parent_namespace, url_pattern.namespace])),
+                    url_pattern.app_name,
                 )
             else:
                 self.all_patterns.append((
-                    parent_url + get_pattern(url_pattern),
+                    parent_url + get_pattern(url_pattern) if parent_url else get_pattern(url_pattern),
                     self.get_lookup_str(url_pattern),
-                    url_pattern.name
+                    url_pattern.name,
+                    parent_namespace,
+                    app_name,
                 ))
 
     @staticmethod
     def get_lookup_str(url_pattern):
         return url_pattern.lookup_str
 
-    def create_tests_for_endpoint(self, url_pattern, url_name):
-        if self.is_endpoint_skipped(url_name):
+    def create_tests_for_endpoint(self, url_pattern, url_name, url_namespace, app_name):
+        if self.is_endpoint_skipped(url_name, url_namespace, app_name):
             self.create_tests_for_http_methods(None, url_pattern, skipped=True)
         else:
             try:
@@ -166,8 +171,19 @@ def create_tests_for_endpoint(self, url_pattern, url_name):
                 self.create_tests_for_http_methods(url, url_pattern, detail_url=bool(url_params))
 
     @staticmethod
-    def is_endpoint_skipped(url_name):
+    def is_endpoint_skipped(url_name, url_namespace, app_name):
         try:
+            if not url_name:
+                return False
+
+            url_name_with_namespace = f'{url_namespace}:{url_name}' if url_namespace else url_name
+            if url_name_with_namespace in settings.SKIP_SMOKE_TESTS:
+                return True
+
+            url_name_with_app_name = f'{app_name}:{url_name}' if app_name else url_name
+            if url_name_with_app_name in settings.SKIP_SMOKE_TESTS:
+                return True
+
             return url_name and url_name in settings.SKIP_SMOKE_TESTS
         except AttributeError:
             return False
diff --git a/tests/another_app/__init__.py b/tests/another_app/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/another_app/urls.py b/tests/another_app/urls.py
new file mode 100644
index 0000000..2ba0149
--- /dev/null
+++ b/tests/another_app/urls.py
@@ -0,0 +1,12 @@
+from django.urls import path
+
+from .views import dummy_view
+
+
+app_name = 'another_app'
+another_app_skipped_urls = [
+    path('skipped-app-endpoint-by-namespace/', dummy_view, name='skipped_endpoint_by_namespace_in_another_app'),
+    path('skipped-app-endpoint-by-app-name/', dummy_view, name='skipped_endpoint_by_app_name_in_another_app'),
+]
+
+urlpatterns = another_app_skipped_urls
\ No newline at end of file
diff --git a/tests/another_app/views.py b/tests/another_app/views.py
new file mode 100644
index 0000000..6de2c74
--- /dev/null
+++ b/tests/another_app/views.py
@@ -0,0 +1,5 @@
+from django.http import HttpResponse
+
+
+def dummy_view(request):
+    return HttpResponse()
diff --git a/tests/settings.py b/tests/settings.py
index d843896..5fd3609 100644
--- a/tests/settings.py
+++ b/tests/settings.py
@@ -26,6 +26,7 @@
     "django_smoke_tests",
 
     'rest_framework',
+    'tests.another_app',
     'tests.app',
 ]
 
@@ -36,6 +37,8 @@
 SKIP_SMOKE_TESTS = (
     'skipped_endpoint',
     'skipped_app_endpoint',
+    'another_app_namespace:skipped_endpoint_by_namespace_in_another_app',
+    'another_app:skipped_endpoint_by_app_name_in_another_app',
 )
 
 if django.VERSION >= (1, 10):
diff --git a/tests/test_generator.py b/tests/test_generator.py
index fdfce66..47ffe25 100644
--- a/tests/test_generator.py
+++ b/tests/test_generator.py
@@ -15,6 +15,7 @@
 from django_smoke_tests.generator import AppNotInInstalledApps, SmokeTestsGenerator, get_pattern
 from django_smoke_tests.runners import NoDbTestRunner
 from django_smoke_tests.tests import SmokeTests
+from tests.another_app.urls import another_app_skipped_urls
 
 from tests.app.urls import urlpatterns as app_url_patterns
 from tests.app.urls import skipped_app_url_patterns
@@ -48,6 +49,11 @@ def flush():
 
 
 class TestSmokeTestsGenerator(TestCase):
+    """
+    These tests rely on the dummy Django project created under tests/ directory.
+    TODO: consider splitting these tests into unit (using mocks) and integration tests (using dummy project)
+    """
+
     def setUp(self):
         super(TestSmokeTestsGenerator, self).setUp()
         self.tests_generator = SmokeTestsGenerator()
@@ -266,7 +272,7 @@ def test_create_skipped_test_for_not_supported_endpoint(self, http_method, mocke
         )
 
         tests_generator.create_tests_for_endpoint(
-            get_pattern(url_pattern), url_pattern.name
+            get_pattern(url_pattern), url_pattern.name, None, None,
         )
         self.assertTrue(
             hasattr(SmokeTests, expected_test_name)
@@ -296,8 +302,11 @@ def test_create_tests_for_endpoint_using_path_with_parameter(
 
         # Create a test for the path loaded above
         loaded_url_pattern = tests_generator.all_patterns[0]
-        tests_generator.create_tests_for_endpoint(
-            loaded_url_pattern[0], loaded_url_pattern[2],  # url_pattern, url_name - logic from execute()
+        tests_generator.create_tests_for_endpoint(  # logic from execute()
+            loaded_url_pattern[0],
+            loaded_url_pattern[2],
+            loaded_url_pattern[3],
+            loaded_url_pattern[4],
         )
 
         # Check if the test is created against a proper URL
@@ -519,6 +528,48 @@ def test_skipped_app_url(self, mocked_call_command):
         self.assertEqual(len(skipped), 1)
         self.assertEqual(failures, [])
 
+    @patch('django_smoke_tests.generator.call_command')
+    def test_skipped_app_url_using_namespace(self, mocked_call_command):
+        url_pattern = another_app_skipped_urls[0]
+        http_method = 'GET'
+        tests_generator = SmokeTestsGenerator(http_methods=[http_method])
+        tests_generator.execute()
+
+        expected_test_name = tests_generator.create_test_name(
+            http_method, '^another_app_urls/' + get_pattern(url_pattern)
+        )
+
+        self.assertTrue(
+            hasattr(SmokeTests, expected_test_name)
+        )
+
+        is_successful, failures, skipped = self._execute_smoke_test(expected_test_name)
+
+        self.assertTrue(is_successful)
+        self.assertEqual(len(skipped), 1)
+        self.assertEqual(failures, [])
+
+    @patch('django_smoke_tests.generator.call_command')
+    def test_skipped_app_url_using_app_name(self, mocked_call_command):
+        url_pattern = another_app_skipped_urls[1]
+        http_method = 'GET'
+        tests_generator = SmokeTestsGenerator(http_methods=[http_method])
+        tests_generator.execute()
+
+        expected_test_name = tests_generator.create_test_name(
+            http_method, '^another_app_urls/' + get_pattern(url_pattern)
+        )
+
+        self.assertTrue(
+            hasattr(SmokeTests, expected_test_name)
+        )
+
+        is_successful, failures, skipped = self._execute_smoke_test(expected_test_name)
+
+        self.assertTrue(is_successful)
+        self.assertEqual(len(skipped), 1)
+        self.assertEqual(failures, [])
+
     @override_settings(SKIP_SMOKE_TESTS=())
     @patch('django_smoke_tests.generator.call_command')
     def test_if_url_is_not_skipped_when_setting_is_empty(self, mocked_call_command):
diff --git a/tests/urls.py b/tests/urls.py
index 34fcef8..046558b 100644
--- a/tests/urls.py
+++ b/tests/urls.py
@@ -36,7 +36,8 @@
     ),
 
     # fixed edge cases
-    url(r'^app_urls/', include('tests.app.urls')),
+    path('app_urls/', include('tests.app.urls')),
+    path('another_app_urls/', include('tests.another_app.urls', namespace='another_app_namespace')),
 
     # using path()
     path(