Perform github path check during validation, rather than metadata par…

…sing. Downstream users of DocGen often load the metadata separately from the original file, which is a problem when the excerpt github link checks against an actual path.
awsdocs · Sep 4, 2024 · d31cf0c · d31cf0c
1 parent 4517bf6
commit d31cf0c
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 56 deletions.
diff --git a/aws_doc_sdk_examples_tools/doc_gen.py b/aws_doc_sdk_examples_tools/doc_gen.py
@@ -256,6 +256,8 @@ def validate(self):
             sdk.validate(self.errors)
         for service in self.services.values():
             service.validate(self.errors)
+        for example in self.examples.values():
+            example.validate(self.errors, self.root)
         validate_metadata(self.root, self.errors)
         validate_no_duplicate_api_examples(self.examples.values(), self.errors)
         validate_snippets(

diff --git a/aws_doc_sdk_examples_tools/metadata.py b/aws_doc_sdk_examples_tools/metadata.py
@@ -73,6 +73,9 @@ def from_yaml(cls, yaml: Any) -> tuple["Excerpt", MetadataErrors]:
 
         return (cls(description, snippet_tags, snippet_files, genai), errors)
 
+    def validate(self, errors: MetadataErrors):
+        pass
+
 
 @dataclass
 class Version:
@@ -112,23 +115,6 @@ def from_yaml(
             if sdkguide.startswith("https://docs.aws.amazon.com"):
                 errors.append(metadata_errors.InvalidSdkGuideStart(guide=sdkguide))
 
-        if github is not None:
-            _, ext = splitext(github)
-            if ext != "":
-                errors.append(
-                    metadata_errors.InvalidGithubLink(
-                        link=github, sdk_version=sdk_version
-                    )
-                )
-            elif github.startswith("http"):
-                pass  # Tributaries specify full GitHub path. Consider passing in GitHub root from tributaries and doing a full check at some point.
-            elif not (root / github).exists():
-                errors.append(
-                    metadata_errors.MissingGithubLink(
-                        link=github, sdk_version=sdk_version, root=root
-                    )
-                )
-
         excerpts = []
         for excerpt in yaml.get("excerpts", []):
             parsed, parse_errors = Excerpt.from_yaml(excerpt)
@@ -173,6 +159,28 @@ def from_yaml(
             errors,
         )
 
+    def validate(self, errors: MetadataErrors, root: Path):
+        github = self.github
+        if github is not None:
+            _, ext = splitext(github)
+            if ext != "":
+                errors.append(
+                    metadata_errors.InvalidGithubLink(
+                        link=github, sdk_version=self.sdk_version
+                    )
+                )
+            elif github.startswith("http"):
+                pass  # Tributaries specify full GitHub path. Consider passing in GitHub root from tributaries and doing a full check at some point.
+            elif not (root / github).exists():
+                errors.append(
+                    metadata_errors.MissingGithubLink(
+                        link=github, sdk_version=self.sdk_version, root=root
+                    )
+                )
+
+        for excerpt in self.excerpts:
+            excerpt.validate(errors)
+
 
 @dataclass
 class Language:
@@ -242,6 +250,15 @@ def from_yaml(
 
         return cls(name, property, versions), errors
 
+    def validate(self, errors: MetadataErrors, root: Path):
+        errs = MetadataErrors()
+        for version in self.versions:
+            version.validate(errs, root)
+        for error in errs:
+            if isinstance(error, MetadataParseError):
+                error.language = self.name
+        errors.extend(errs)
+
 
 @dataclass
 class Example:
@@ -394,6 +411,15 @@ def from_yaml(
             errors,
         )
 
+    def validate(self, errors: MetadataErrors, root: Path):
+        errs = MetadataErrors()
+        for language in self.languages.values():
+            language.validate(errs, root)
+        for error in errs:
+            error.file = self.file
+            error.id = self.id
+        errors.extend(errs)
+
 
 def parse_services(
     yaml: Any, errors: MetadataErrors, known_services: Dict[str, Service]

diff --git a/aws_doc_sdk_examples_tools/metadata_test.py b/aws_doc_sdk_examples_tools/metadata_test.py
@@ -8,7 +8,7 @@
 import pytest
 import yaml
 from pathlib import Path
-from typing import List, Set, Tuple, Optional
+from typing import List, Set, Tuple
 
 from . import metadata_errors
 from .metadata_errors import MetadataErrors, ExampleMergeConflict
@@ -32,13 +32,9 @@
 def load(
     path: Path, doc_gen: DocGen, blocks: Set[str] = set()
 ) -> Tuple[List[Example], metadata_errors.MetadataErrors]:
-    root = Path(__file__).parent
-    filename = root / "test_resources" / path
-    with filename.open() as file:
+    with path.open() as file:
         meta = yaml.safe_load(file)
-    return parse(
-        filename, meta, doc_gen.sdks, doc_gen.services, blocks, doc_gen.validation
-    )
+    return parse(path, meta, doc_gen.sdks, doc_gen.services, blocks, doc_gen.validation)
 
 
 SERVICES = {
@@ -378,14 +374,6 @@ def test_parse_strict_title_errors():
             file=Path("test_cpp.yaml"),
             id="medical-imaging_BadBasics",
         ),
-        metadata_errors.MissingGithubLink(
-            file=Path("test_cpp.yaml"),
-            id="medical-imaging_BadBasics",
-            language="C++",
-            sdk_version=1,
-            link="cpp/example_code/medical-imaging",
-            root=Path("."),
-        ),
     ]
     assert expected == [*errors]
 
@@ -464,7 +452,7 @@ def test_parse_cross():
 
 def test_verify_load_successful():
     actual, errors = load(
-        Path(__file__).parent / "test_resources/valid_metadata.yaml",
+        TEST_RESOURCES_PATH / "valid_metadata.yaml",
         DOC_GEN,
         set(["test block"]),
     )
@@ -595,15 +583,14 @@ def test_verify_load_successful():
     assert actual[0] == example
 
 
-EMPTY_METADATA_PATH = Path(__file__).parent / "test_resources/empty_metadata.yaml"
-ERRORS_METADATA_PATH = Path(__file__).parent / "test_resources/errors_metadata.yaml"
-FORMATTER_METADATA_PATH = (
-    Path(__file__).parent / "test_resources/formaterror_metadata.yaml"
-)
+TEST_RESOURCES_PATH = Path(__file__).parent / "test_resources"
+EMPTY_METADATA_PATH = TEST_RESOURCES_PATH / "empty_metadata.yaml"
+ERRORS_METADATA_PATH = TEST_RESOURCES_PATH / "errors_metadata.yaml"
+FORMATTER_METADATA_PATH = TEST_RESOURCES_PATH / "formaterror_metadata.yaml"
 
 
 @pytest.mark.parametrize(
-    "filename,expected_errors",
+    "filename,expected_errors,validation_errors",
     [
         (
             "empty_metadata.yaml",
@@ -620,6 +607,7 @@ def test_verify_load_successful():
                     svcs=[],
                 ),
             ],
+            [],
         ),
         (
             "errors_metadata.yaml",
@@ -641,14 +629,6 @@ def test_verify_load_successful():
                     guide="https://docs.aws.amazon.com/absolute/link-to-my-guide",
                     sdk_version=1,
                 ),
-                metadata_errors.MissingGithubLink(
-                    file=ERRORS_METADATA_PATH,
-                    id="sqs_WrongServiceSlug",
-                    language="Perl",
-                    sdk_version=1,
-                    link="perl/example_code/medical-imaging",
-                    root=ERRORS_METADATA_PATH.parent,
-                ),
                 metadata_errors.MissingBlockContentAndExcerpt(
                     file=ERRORS_METADATA_PATH,
                     id="sqs_WrongServiceSlug",
@@ -691,13 +671,6 @@ def test_verify_load_successful():
                     id="medical-imaging_TestExample2",
                     service="garbled",
                 ),
-                metadata_errors.InvalidGithubLink(
-                    file=ERRORS_METADATA_PATH,
-                    id="medical-imaging_TestExample2",
-                    language="Java",
-                    sdk_version=2,
-                    link="github/link/to/README.md",
-                ),
                 metadata_errors.FieldError(
                     file=ERRORS_METADATA_PATH,
                     id="medical-imaging_TestExample2",
@@ -730,6 +703,23 @@ def test_verify_load_successful():
                     id="medical-imagingBadFormat",
                 ),
             ],
+            [
+                metadata_errors.MissingGithubLink(
+                    file=ERRORS_METADATA_PATH,
+                    id="sqs_WrongServiceSlug",
+                    language="Perl",
+                    sdk_version=1,
+                    link="perl/example_code/medical-imaging",
+                    root=TEST_RESOURCES_PATH,
+                ),
+                metadata_errors.InvalidGithubLink(
+                    file=ERRORS_METADATA_PATH,
+                    id="medical-imaging_TestExample2",
+                    language="Java",
+                    sdk_version=2,
+                    link="github/link/to/README.md",
+                ),
+            ],
         ),
         (
             "formaterror_metadata.yaml",
@@ -746,14 +736,22 @@ def test_verify_load_successful():
                     service="garbage",
                 ),
             ],
+            [],
         ),
     ],
 )
 def test_common_errors(
-    filename: str, expected_errors: List[metadata_errors.MetadataError]
+    filename: str,
+    expected_errors: List[metadata_errors.MetadataError],
+    validation_errors: List[metadata_errors.MetadataError],
 ):
-    _, actual = load(Path(filename), DOC_GEN, set(["test/block", "cross_block.xml"]))
+    root = TEST_RESOURCES_PATH / filename
+    examples, actual = load(root, DOC_GEN, set(["test/block", "cross_block.xml"]))
     assert expected_errors == [*actual]
+    validations = MetadataErrors()
+    for example in examples:
+        example.validate(validations, root.parent)
+    assert validation_errors == [*validations]
 
 
 TEST_SERVICES = {"test": {"Test", "Test2", "Test3", "1"}}