From 931ea71b119237d88dce7aa7c07aa81eca7ee78e Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Wed, 27 Sep 2023 16:53:25 -0700 Subject: [PATCH 1/2] Add test for application descriptions Check that the application documentation page titles match our standard format and start with a capital letter. This should be handled automatically by the phalanx application create command going forward. --- .../kubernetes-replicator/index.rst | 2 +- docs/applications/linters/index.rst | 6 ++--- docs/applications/obsloctap/index.rst | 5 ++-- .../onepassword-connect-dev/index.rst | 2 +- tests/docs/applications_test.py | 25 +++++++++++++++++++ 5 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 tests/docs/applications_test.py diff --git a/docs/applications/kubernetes-replicator/index.rst b/docs/applications/kubernetes-replicator/index.rst index 89bf636092..f7eeb2ccdd 100644 --- a/docs/applications/kubernetes-replicator/index.rst +++ b/docs/applications/kubernetes-replicator/index.rst @@ -1,7 +1,7 @@ .. px-app:: kubernetes-replicator ################################################# -kubernetes-replicator - Cross-namespace resources +kubernetes-replicator — Cross-namespace resources ################################################# kubernetes-replicator is a Kubernetes operator that replicates resources across namespaces. diff --git a/docs/applications/linters/index.rst b/docs/applications/linters/index.rst index e91f97b9c2..e055a99ede 100644 --- a/docs/applications/linters/index.rst +++ b/docs/applications/linters/index.rst @@ -1,8 +1,8 @@ .. px-app:: linters -###################################### -linters - automated chechking of DNS -###################################### +#################################### +linters — Automated chechking of DNS +#################################### Linters provides a way to automatically and repeatedly check things in ops, such as if DNS entries are pointing to IP addresses that we are using, or are they dangling. We use the route53 API diff --git a/docs/applications/obsloctap/index.rst b/docs/applications/obsloctap/index.rst index 889cf8770c..2de2285249 100644 --- a/docs/applications/obsloctap/index.rst +++ b/docs/applications/obsloctap/index.rst @@ -1,16 +1,15 @@ .. px-app:: obsloctap #################################### -obsloctap — serve observing schedule +obsloctap — Serve observing schedule #################################### Lookup and reformat ``lsst.sal.Scheduler.logevent_predictedSchedule``. -Return a json file of the future observations. +Return a JSON file of the future observations. Todo: Also track which observations were made implement ObsLocTAP_ IVOA standard. .. _ObsLocTAP: https://www.ivoa.net/documents/ObsLocTAP/ - .. jinja:: obsloctap :file: applications/_summary.rst.jinja diff --git a/docs/applications/onepassword-connect-dev/index.rst b/docs/applications/onepassword-connect-dev/index.rst index 24462e04e6..4961a87c67 100644 --- a/docs/applications/onepassword-connect-dev/index.rst +++ b/docs/applications/onepassword-connect-dev/index.rst @@ -1,7 +1,7 @@ .. px-app:: onepassword-connect-dev #################################################### -onepassword-connect-dev - 1Password API server (dev) +onepassword-connect-dev — 1Password API server (dev) #################################################### 1Password Connect provides API access to a 1Password vault. diff --git a/tests/docs/applications_test.py b/tests/docs/applications_test.py new file mode 100644 index 0000000000..a3ec54fc13 --- /dev/null +++ b/tests/docs/applications_test.py @@ -0,0 +1,25 @@ +"""Tests for the application documentation pages.""" + +from __future__ import annotations + +import re +from pathlib import Path + + +def test_descriptions() -> None: + """Ensure all application pages have proper short descriptions.""" + doc_root = Path(__file__).parent.parent.parent / "docs" / "applications" + for application in doc_root.iterdir(): + if not application.is_dir(): + continue + index_path = application / "index.rst" + index_rst = index_path.read_text() + m = re.search(r"\n(#+)\n([^\n]+)\n\1\n", index_rst) + assert m, f"No title found in {index_path}" + title = m.group(2) + assert len(title) <= 80, f"Title too long in {index_path}" + m = re.match(r"\S+ — (\S.*$)", title) + assert m, f"Invalid title format in {index_path}" + description = m.group(1) + m = re.match("[A-Z0-9]", description) + assert m, f"Description must start with capital letter in {index_path}" From f376463ee1c20e6c126f4986368a7ddaecfcb011 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Wed, 27 Sep 2023 17:03:52 -0700 Subject: [PATCH 2/2] Check description in Phalanx CLI Check that the description starts with a capital letter and is not excessively long inside the Phalanx CLI so that it won't create new applications that immediately fail the application docs test. --- src/phalanx/cli.py | 10 +++++++++- tests/cli/application_test.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/phalanx/cli.py b/src/phalanx/cli.py index 9d82bfd2ac..2fc2b971b2 100644 --- a/src/phalanx/cli.py +++ b/src/phalanx/cli.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re import sys from pathlib import Path @@ -121,7 +122,10 @@ def application() -> None: "-d", "--description", prompt="Short description", - help="Short description of the new application.", + help=( + "Short description of the new application. Must start with capital" + " letter and, with the application name, be less than 80 characters." + ), ) @click.option( "-s", @@ -142,6 +146,10 @@ def application_create( """ if not config: config = _find_config() + if len(name) + 3 + len(description) > 80: + raise click.UsageError("Name plus description is too long") + if not re.match("[A-Z0-9]", description): + raise click.UsageError("Description must start with capital letter") factory = Factory(config) application_service = factory.create_application_service() application_service.create_application( diff --git a/tests/cli/application_test.py b/tests/cli/application_test.py index b19be99328..0048775d99 100644 --- a/tests/cli/application_test.py +++ b/tests/cli/application_test.py @@ -117,6 +117,35 @@ def test_create(tmp_path: Path) -> None: assert environment.applications["hips"].chart["sources"][0] == expected +def test_create_errors(tmp_path: Path) -> None: + config_path = tmp_path / "phalanx" + shutil.copytree(str(phalanx_test_path()), str(config_path)) + result = run_cli( + "application", + "create", + "some-really-long-app-name-please-do-not-do-this", + "--description", + "Some really long description on top of the app name", + "--config", + str(config_path), + needs_config=False, + ) + assert "Name plus description is too long" in result.output + assert result.exit_code == 2 + result = run_cli( + "application", + "create", + "app", + "--description", + "lowercase description", + "--config", + str(config_path), + needs_config=False, + ) + assert "Description must start with capital letter" in result.output + assert result.exit_code == 2 + + def test_create_prompt(tmp_path: Path) -> None: config_path = tmp_path / "phalanx" shutil.copytree(str(phalanx_test_path()), str(config_path))