Conversation
Hard to argue that unit tests for an extension provide any meaningful coverage for a tool requirement.
a-zw
requested review from
AlexanderLanin,
MaximilianSoerenPollak,
dcalavrezo-qorix and
nradakovic
as code owners
September 30, 2026 11:01
Contributor
|
Documentation preview for this pull request is available at: |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Several verification mappings overstate coverage, one fixture uses an invalid link type, and malformed milestones can still trigger misleading consistency warnings.
Review effort: Balanced
Findings: 5
Open (6)
What changed in this PR
Expands metamodel requirement verification and improves malformed milestone handling.
Changes:
- Adds and broadens RST-based requirement tests.
- Reclassifies verification metadata.
- Prevents some invalid milestones from crashing consistency checks.
| File | Description |
|---|---|
tests/test_metamodel__init__.py |
Updates unit-test metadata. |
tests/test_check_options.py |
Updates option-test metadata. |
tests/rst/safety/test_saf_violates.rst |
Expands safety-link tests. |
tests/rst/safety/test_saf_mandatory_attrs.rst |
Updates mandatory-attribute coverage. |
tests/rst/options/test_options_options.rst |
Adds status and security cases. |
tests/rst/graph/test_metamodel_graph.rst |
Updates graph safety coverage. |
tests/rst/graph/test_arch_link_safety.rst |
Adds architecture safety-link tests. |
tests/rst/attributes/test_validity.rst |
Expands milestone validation tests. |
tests/rst/attributes/test_prohibited_words.rst |
Covers additional prohibited words. |
tests/rst/attributes/test_common_attr_description.rst |
Adds description-presence tests. |
tests/rst/attributes/test_attributes_format_id_format.rst |
Reclassifies ID-format verification. |
tests/rst/architecture/test_arch_link_fulfils.rst |
Adds architecture fulfilment tests. |
checks/check_options.py |
Handles milestone parsing failures. |
💡 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.
Copilot review overview
🟡 Changes recommended
Several requirements are marked fully verified despite uncovered directive-specific behavior, including an untested component safety-link path.
Review effort: Balanced
Findings: 5
Open (5)
Resolved since last review (6)
|
|
||
| .. test_metadata:: | ||
| :id: test_metadata__arch_link_fulfils | ||
| :fully_verifies_list: tool_req__docs_arch_link_fulfils[version==3] |
| :partially_verifies_list: tool_req__docs_req_attr_validity_correctness,tool_req__docs_req_attr_validity_consistency | ||
| .. test_metadata:: Validity attribute checks | ||
| :id: test_metadata__validity_checks | ||
| :fully_verifies_list: tool_req__docs_req_attr_validity_correctness, tool_req__docs_req_attr_validity_consistency |
| .. test_metadata:: | ||
| :id: test_metadata__saf_violates | ||
| :partially_verifies_list: tool_req__docs_saf_attrs_violates[version==2] | ||
| :fully_verifies_list: tool_req__docs_saf_attrs_violates[version==2] |
AlexanderLanin
previously approved these changes
Oct 2, 2026
| :id: test_metadata__metamodel_graph_checks | ||
| :fully_verifies_list: potential_tool_malfunction__docs_as_code__m2 | ||
| :partially_verifies_list: tool_req__docs_common_attr_safety_link_check | ||
| :fully_verifies_list: potential_tool_malfunction__docs_as_code__m2, |
Member
There was a problem hiding this comment.
follow up cleanup: test cases shall not link malfunctions
Member
|
See copilot feedback, it looks correct |
a-zw
marked this pull request as draft
October 5, 2026 07:50
Contributor
Author
|
This pull request exploded into too much. I'll split it into multiple ones, so one can actually review it:
Dropping the rest of this PR to start that from scratch and closing now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Description
Some tests are already "fully" verifying and some just needed a little bit more.
🚨 Impact Analysis
✅ Checklist