help: Restore broken link checks for help center documentation.

We add a step to build help center and then test the broken links as we
used to before removing the test temporarily.

This commit focuses on just adding back the broken link checks for the
help center. We skip the fragment check since that is in-built in
starlight and starlight tests account for that already. For the image
check we can add it back in a followup issue.

.github/workflows/zulip-ci.yml

@@ -121,9 +121,10 @@ jobs:
         if: ${{ matrix.include_documentation_tests }}
         run: |
           source tools/ci/activate-venv
+          ./tools/build-help-center
           # In CI, we only test links we control in test-documentation to avoid flakes
           ./tools/test-documentation --skip-external-links
-          ./tools/test-help-documentation --skip-external-links
+          ./tools/test-help-documentation --skip-external-links --help-center
           ./tools/test-api
 
       - name: Run node tests

tools/documentation_crawler/documentation_crawler/spiders/check_help_documentation.py

@@ -42,6 +42,13 @@ class UnusedImagesLinterSpider(BaseDocumentationSpider):
             self.logger.error(exception_message.format(", ".join(unused_images_relatedpath)))
 
 
+class HelpDocumentationSpider(BaseDocumentationSpider):
+    name = "help_documentation_crawler"
+    start_urls = ["http://localhost:9981/help"]
+    deny_domains: list[str] = []
+    deny = ["/policies/privacy"]
+
+
 class APIDocumentationSpider(UnusedImagesLinterSpider):
     name = "api_documentation_crawler"
     start_urls = ["http://localhost:9981/api"]
@@ -54,7 +61,7 @@ class PorticoDocumentationSpider(BaseDocumentationSpider):
     def _is_external_url(self, url: str) -> bool:
         return (
             not url.startswith("http://localhost:9981")
-            or url.startswith("http://localhost:9981/api")
+            or url.startswith(("http://localhost:9981/help", "http://localhost:9981/api"))
             or self._has_extension(url)
         )
 

tools/documentation_crawler/documentation_crawler/spiders/common/spiders.py

@@ -154,10 +154,6 @@ class BaseDocumentationSpider(scrapy.Spider):
             return
         if url.startswith("http://localhost:9981/plans"):
             return
-        # We are temporarily stopping these tests while we cutover to
-        # the new help center. https://github.com/zulip/zulip/issues/35647.
-        if url.startswith("http://localhost:9981/help"):
-            return
 
         callback: Callable[[Response], Iterator[Request] | None] = self.parse
         dont_filter = False
@@ -188,7 +184,7 @@ class BaseDocumentationSpider(scrapy.Spider):
                         "There is no local directory associated with the GitHub URL: %s", url
                     )
                 return
-        elif split_url.fragment != "":
+        elif getattr(self, "skip_check_fragment", False) and split_url.fragment != "":
             dont_filter = True
             callback = self.check_fragment
         if getattr(self, "skip_external", False) and self._is_external_link(url):

tools/lib/test_server.py

@@ -57,6 +57,7 @@ def test_server_running(
     external_host: str = "testserver",
     log_file: str | None = None,
     dots: bool = False,
+    enable_help_center: bool = False,
 ) -> Iterator[None]:
     with ExitStack() as stack:
         log = sys.stdout
@@ -75,6 +76,8 @@ def test_server_running(
         run_dev_server_command = ["tools/run-dev", "--test", "--streamlined"]
         if skip_provision_check:
             run_dev_server_command.append("--skip-provision-check")
+        if enable_help_center:
+            run_dev_server_command.append("--help-center")
         server = subprocess.Popen(run_dev_server_command, stdout=log, stderr=log)
 
         try:

tools/test-help-documentation

@@ -26,6 +26,12 @@ parser.add_argument(
     action="store_true",
     help="Skip checking of external links.",
 )
+parser.add_argument(
+    "--help-center",
+    dest="enable_help_center",
+    action="store_true",
+    help="Test help center generated as part of the astro build process. If this option is set to True, this script assumes that you already have performend the build step.",
+)
 options = parser.parse_args()
 
 os.makedirs("var/help-documentation", exist_ok=True)
@@ -33,7 +39,7 @@ os.makedirs("var/help-documentation", exist_ok=True)
 LOG_FILE = "var/help-documentation/server.log"
 external_host = "localhost:9981"
 
-extra_args = ["-a", "validate_html=set"]
+extra_args = []
 
 if options.skip_external_link_check:
     extra_args += ["-a", "skip_external=set"]
@@ -60,8 +66,26 @@ def vnu_servlet() -> Iterator[None]:
 
 with (
     vnu_servlet(),
-    test_server_running(options.skip_provision_check, external_host, log_file=LOG_FILE, dots=True),
+    test_server_running(
+        options.skip_provision_check,
+        external_host,
+        log_file=LOG_FILE,
+        dots=True,
+        enable_help_center=options.enable_help_center,
+    ),
 ):
+    ret_help_doc = subprocess.call(
+        [
+            "scrapy",
+            "crawl_with_status",
+            *extra_args,
+            "-a",
+            "skip_check_fragment=set",
+            "help_documentation_crawler",
+        ],
+        cwd="tools/documentation_crawler",
+    )
+    extra_args += ["-a", "validate_html=set"]
     ret_api_doc = subprocess.call(
         ["scrapy", "crawl_with_status", *extra_args, "api_documentation_crawler"],
         cwd="tools/documentation_crawler",
@@ -71,7 +95,7 @@ with (
         cwd="tools/documentation_crawler",
     )
 
-if ret_api_doc != 0 or ret_portico_doc != 0:
+if ret_help_doc != 0 or ret_api_doc != 0 or ret_portico_doc != 0:
     print("\033[0;91m")
     print("Failed")
     print("\033[0m")
@@ -81,4 +105,4 @@ else:
     print("\033[0m")
 
 
-sys.exit(ret_api_doc or ret_portico_doc)
+sys.exit(ret_help_doc or ret_api_doc or ret_portico_doc)

zerver/lib/test_helpers.py

@@ -570,15 +570,6 @@ 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:
@@ -587,7 +578,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 untested_patterns:  # nocoverage -- test suite error handling
+        if full_suite and len(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}")