From 9ec15e99bd30064d9695864bb34e95270ebef6e4 Mon Sep 17 00:00:00 2001 From: Shubham Padia Date: Fri, 8 Aug 2025 08:55:28 +0000 Subject: [PATCH] test_urls: Use /api instead of /help pages for some tests. We are going to move the help center to starlight soon and the tests won't be able to access that help center until we solve things in a followup issue. We move some of the urls being tested from /help to /api to not loose coverage on those tests. --- zerver/lib/test_helpers.py | 11 ++++++++- zerver/tests/test_urls.py | 42 +++++++++++++++-------------------- zerver/views/documentation.py | 7 +++++- 3 files changed, 34 insertions(+), 26 deletions(-) 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):