Copilot/awesome ai drivendevelopment 101 - #24
Conversation
…test-run-develop-repeat Add repository-wide PoC validation and regression coverage
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: 5hy7xz92nd-oss <249378113+5hy7xz92nd-oss@users.noreply.github.com>
Co-authored-by: 5hy7xz92nd-oss <249378113+5hy7xz92nd-oss@users.noreply.github.com>
Co-authored-by: 5hy7xz92nd-oss <249378113+5hy7xz92nd-oss@users.noreply.github.com>
…phic-blueprint
…ck-merge-into-13
…zed-architecture
…evelopment-101
Co-authored-by: 5hy7xz92nd-oss <249378113+5hy7xz92nd-oss@users.noreply.github.com>
Co-authored-by: 5hy7xz92nd-oss <249378113+5hy7xz92nd-oss@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request introduces a repository-wide validation tool to sanity-check PoC entrypoints/manifests and ensure the root catalog stays consistent with the on-disk PoC directories, and wires those checks into CI.
Changes:
- Add
validate_pocs.pyto discover and validate Python/JS/JSON manifests plus README/catalog consistency. - Add
tests/test_validate_pocs.pyto exercise discovery/validation and fixture behavior. - Document the validation workflow in
README.md, add CI workflow, and expand.gitignore.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| validate_pocs.py | New validator that discovers PoC-related files and performs syntax/manifest/catalog checks. |
| tests/test_validate_pocs.py | Unit tests for discovery + repository/fixture validation behavior. |
| README.md | Adds a “Validation” section describing how to run checks locally and in CI. |
| .gitignore | Ignores common Python/Node/test artifacts (caches, venvs, coverage, etc.). |
| .github/workflows/validate.yml | CI job to run unit tests and repository validation on pushes/PRs. |
| .github/instructions/*.instructions.md | Adds Copilot instruction content under .github/instructions/. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
tests/test_validate_pocs.py:37
- This test also asserts minimum file/directory counts for the whole repository, which will fail as soon as the repo structure changes (even if validation still works). Prefer asserting only that validation succeeds and that the counts are non-zero where appropriate.
self.assertEqual(result["errors"], [])
self.assertGreater(result["python_files"], 20)
self.assertGreater(result["package_json_files"], 0)
self.assertGreaterEqual(result["cargo_toml_files"], 2)
self.assertGreater(result["javascript_files"], 0)
.github/instructions/*.instructions.md:5
- This Copilot instructions file appears to contain large amounts of unrelated ASCII/emoji art and copied PR discussion content rather than actionable repository instructions. Also, the filename is literally ".instructions.md" (contains ''), which likely prevents GitHub from recognizing it as an instructions file. Please replace it with concise instructions in a properly named file (e.g. review.instructions.md) or remove it.
@5hy7xz92nd-oss @copilot @copilot @copilot @we-tech-company @copilot @5hy7xz92nd-oss 🕴️🔃〰️⤴️@copilot🌎 REALITY
|
↓
👁️ Observer
|
validate_pocs.py:182
- The requirements.txt validation flags any option line that starts with '-' and contains no alphanumeric characters as invalid. This will incorrectly reject valid pip option entries like "-e ." (editable install) or path-based options. Consider only rejecting clearly empty option markers ("-"/"--") or simply allowing option lines.
for line in meaningful:
if line.startswith("-") and not any(ch.isalnum() for ch in line):
errors.append(f"{path.relative_to(root)}: invalid requirement line: {line!r}")
tests/test_validate_pocs.py:29
- These assertions hardcode specific repository contents (e.g. requiring a file named run_demo.py and minimum counts). That makes the test suite brittle when PoCs are added/removed or renamed, even though discovery could still be working correctly.
This issue also appears on line 33 of the same file.
self.assertTrue(any(path.name == "run_demo.py" for path in python_targets))
self.assertTrue(any(path.name == "package.json" for path in package_targets))
self.assertTrue(any(path.name == "Cargo.toml" for path in cargo_targets))
self.assertTrue(any(path.suffix == ".js" for path in js_targets))
self.assertFalse(any(path.name == "package.json" for path in json_targets))
.github/workflows/validate.yml:32
- The step name says "Validate repository integrations" but this workflow runs repository integrity validation (python validate_pocs.py). Renaming improves clarity and avoids confusion with integration tests.
- name: Validate repository integrations
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/instructions/*.instructions.md:5
- This file is committed with a literal
*in its filename (.github/instructions/*.instructions.md). Filenames containing glob metacharacters commonly break shell tooling, scripts, and some editor integrations, and it also makes the intent of the instructions unclear. Consider renaming it to a concrete, descriptive filename (e.g.review.instructions.md) and trimming the content to actionable Copilot/repo guidance.
@5hy7xz92nd-oss @copilot @copilot @copilot @we-tech-company @copilot @5hy7xz92nd-oss 🕴️🔃〰️⤴️@copilot🌎 REALITY
|
↓
👁️ Observer
|
validate_pocs.py:14
tomllibis only available on Python 3.11+, so running this script with olderpython3(e.g. 3.10) will fail at import time. Either document the minimum Python version explicitly or add a fallback totomliwith a clearer error message so local validation doesn't break unexpectedly.
import subprocess
import sys
import tomllib
from collections import Counter
README.md:83
- The local run instructions don't mention that validation (and the unit tests that call
validate_repo) require Node.js to be installed, and the validator currently implies Python 3.11+ due totomllib. Adding a short prerequisite note here will prevent confusing local failures.
Run locally:
```bash
python -m unittest discover -s tests -v
python validate_pocs.py
</details>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (6)
tests/test_validate_pocs.py:38
- This test also relies on large, repo-specific minimum counts that will become flaky as PoCs are added/removed. Consider asserting smaller lower bounds (or only the invariants you actually need here) and rely on the fixture-based tests for detailed validation behavior.
self.assertGreater(result["python_files"], 20)
self.assertGreater(result["package_json_files"], 0)
self.assertGreaterEqual(result["cargo_toml_files"], 2)
self.assertGreater(result["javascript_files"], 0)
self.assertGreater(result["poc_directories"], 30)
tests/test_validate_pocs.py:29
- These hard-coded minimum counts (e.g., "> 20" Python files, "> 30" PoC dirs) are likely to fail as the repository contents change. Using small, intent-focused lower bounds keeps the test meaningful without being overly sensitive to future additions/removals.
self.assertGreater(len(python_targets), 20)
self.assertGreater(len(package_targets), 0)
self.assertGreaterEqual(len(cargo_targets), 2)
self.assertGreater(len(poc_dirs), 30)
.github/workflows/validate.yml:32
- The step name says "integrations", but this workflow is running repository integrity checks. Renaming avoids confusion when scanning workflow logs.
- name: Validate repository integrations
.github/instructions/*.instructions.md:243
- This file looks like an accidental paste of PR timeline / Copilot review output (and contains a large amount of non-instruction content). Also, the filename contains
*, which is invalid on Windows and can break checkouts and glob-based tooling. Please remove this file or replace it with a concise Copilot instructions document using a safe filename (e.g.,.github/instructions/copilot.instructions.md).
👁️ OBSERVE AGAIN 👁️ OBSERVE AGAIN 🌎 REALITY Reality provides signals.
@5hy7xz92nd-oss
5hy7xz92nd-oss marked this pull request as ready for review last week
5hy7xz92nd-oss
5hy7xz92nd-oss commented last week
tests/test_validate_pocs.py:25
- The discovery test is currently tied to specific repository files and content (e.g.,
run_demo.py). That makes the test brittle as PoC folders evolve. Prefer asserting stable invariants introduced by this PR (e.g.,validate_pocs.pyis discoverable) and basic non-emptiness for key target lists.
This issue also appears in the following locations of the same file:
- line 26
- line 34
self.assertTrue(any(path.name == "run_demo.py" for path in python_targets))
self.assertTrue(any(path.name == "package.json" for path in package_targets))
self.assertTrue(any(path.name == "Cargo.toml" for path in cargo_targets))
self.assertTrue(any(path.suffix == ".js" for path in js_targets))
self.assertFalse(any(path.name == "package.json" for path in json_targets))
tests/test_validate_pocs.py:182
validate_pocs.main()prints to stdout/stderr, so calling it directly in tests makes the unittest output noisy. Redirect stdout/stderr in this test so failures remain readable.
def test_main_success_and_failure_exit_codes(self) -> None:
self.assertEqual(validate_pocs.main(["--root", str(self.repo_root)]), 0)
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
No description provided.