diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index 65e5d5763f..20ddbc579d 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -570,6 +570,15 @@ def write_instrumentation_reports(full_suite: bool, include_webhooks: bool) -> N untested_patterns -= exempt_patterns + # A lot of help/ URLs are part of the untested patterns, which + # is due to the switch to the new help center. We exempt them + # programmatically below. + untested_patterns = { + untested_pattern + for untested_pattern in untested_patterns + if not untested_pattern.startswith("help/") + } + var_dir = "var" # TODO make sure path is robust here fn = os.path.join(var_dir, "url_coverage.txt") with open(fn, "wb") as f: @@ -578,7 +587,7 @@ def write_instrumentation_reports(full_suite: bool, include_webhooks: bool) -> N if full_suite: print(f"INFO: URL coverage report is in {fn}") - if full_suite and len(untested_patterns): # nocoverage -- test suite error handling + if full_suite and untested_patterns: # nocoverage -- test suite error handling print("\nERROR: Some URLs are untested! Here's the list of untested URLs:") for untested_pattern in sorted(untested_patterns): print(f" {untested_pattern}") diff --git a/zerver/tests/test_urls.py b/zerver/tests/test_urls.py index c05f4bb1b6..a279b96533 100644 --- a/zerver/tests/test_urls.py +++ b/zerver/tests/test_urls.py @@ -6,7 +6,6 @@ from django.test import Client from zerver.lib.test_classes import ZulipTestCase from zerver.lib.url_redirects import ( API_DOCUMENTATION_REDIRECTS, - HELP_DOCUMENTATION_REDIRECTS, LANDING_PAGE_REDIRECTS, POLICY_DOCUMENTATION_REDIRECTS, ) @@ -29,27 +28,27 @@ class PublicURLTest(ZulipTestCase): msg=f"Expected {expected_status}, received {response.status_code} for {method} to {url}", ) - def test_help_pages(self) -> None: - # Test all files in help documentation directory (except for 'index.md', - # 'missing.md' and `help/include/` files). + def test_api_doc_pages(self) -> None: + # Test all files in api_docs documentation directory (except for 'index.md', + # 'missing.md', "api-doc-template.md" and `api_docs/include/` files). - help_urls = [] - for doc in os.listdir("./help/"): + api_doc_urls = [] + for doc in os.listdir("./api_docs/"): if doc.startswith(".") or "~" in doc or "#" in doc: continue # nocoverage -- just here for convenience - if doc in {"index.md", "include", "missing.md"}: + if doc in {"index.md", "include", "missing.md", "api-doc-template.md"}: continue - url = "/help/" + os.path.splitext(doc)[0] # Strip the extension. - help_urls.append(url) + url = "/api/" + os.path.splitext(doc)[0] # Strip the extension. + api_doc_urls.append(url) - # We have lots of help files, so this will be expensive! - self.assertGreater(len(help_urls), 190) + # We have lots of api_docs files, so this will be expensive! + self.assertGreater(len(api_doc_urls), 25) - expected_tag = """""" + expected_tag = """""" - for url in help_urls: + for url in api_doc_urls: with mock.patch( - "zerver.lib.html_to_text.html_to_text", return_value="This is a help page" + "zerver.lib.html_to_text.html_to_text", return_value="This is an API docs page" ) as m: response = self.client_get(url) m.assert_called_once() @@ -84,7 +83,7 @@ class PublicURLTest(ZulipTestCase): "/ru/accounts/home/", "/en/accounts/login/", "/ru/accounts/login/", - "/help/", + "/api/", # Since web-public streams are enabled in this `zulip` # instance, the public access experience is loaded directly. "/", @@ -101,10 +100,10 @@ class PublicURLTest(ZulipTestCase): "/api/v1/streams", ], 404: [ - "/help/api-doc-template", - "/help/nonexistent", - "/help/include/admin", - "/help/" + "z" * 1000, + "/api/api-doc-template", + "/api/nonexistent", + "/api/include/admin", + "/api/" + "z" * 1000, ], } @@ -187,11 +186,6 @@ class RedirectURLTest(ZulipTestCase): result = self.client_get(redirect.old_url, follow=True) self.assert_in_success_response(["Zulip homepage", "API documentation home"], result) - def test_help_redirects(self) -> None: - for redirect in HELP_DOCUMENTATION_REDIRECTS: - result = self.client_get(redirect.old_url, follow=True) - self.assert_in_success_response(["Zulip homepage", "Help center home"], result) - def test_policy_redirects(self) -> None: for redirect in POLICY_DOCUMENTATION_REDIRECTS: result = self.client_get(redirect.old_url, follow=True) diff --git a/zerver/views/documentation.py b/zerver/views/documentation.py index b3aa10f44a..fbcda7e3c3 100644 --- a/zerver/views/documentation.py +++ b/zerver/views/documentation.py @@ -106,7 +106,12 @@ class MarkdownDirectoryView(ApiURLView): # This markdown template shouldn't be accessed directly. article = "missing" http_status = 404 - elif "/" in article: + # Only help center allows nested paths in urls.py, once we + # remove that help center url declaration in urls.py after + # switching to the new help center, we should remove this elif + # block altogether since api docs and policies only allow slugs + # which cannot have nested paths. + elif "/" in article: # nocoverage article = "missing" http_status = 404 elif len(article) > 100 or not re.match(r"^[0-9a-zA-Z_-]+$", article):