result: link tests to requirements - #579
Open
Nikhil2206 wants to merge 6 commits into
Open
Nikhil2206 wants to merge 6 commits into
Nikhil2206 wants to merge 6 commits into
Conversation
Nikhil2206
requested review from
4og,
antonkri,
arkjedrz,
fbaeuerle and
pawelrutkaq
as code owners
September 11, 2026 07:54
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 07:54 — with 
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 07:54 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 07:54 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 07:54 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
github-actions
Bot
added
comp-result
Contributor
|
Documentation preview for this pull request is available at: |
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 08:24 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 08:24 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 08:24 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 11, 2026 08:24 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
marked this pull request as draft
September 11, 2026 08:24
…_integration wording comp_req__result__std_integration's normative text only covers conversion to std::optional<T>. Twelve extensions_test.cpp tests were linked to it despite testing score::cpp::expected and score::cpp::optional interop, which the requirement does not describe. Remove the mismatched PartiallyVerifies links; Description/TestType/DerivationTechnique remain accurate test-level metadata. The 4 tests actually converting to/from std::optional keep their existing link.
…tensions tests comp_req__result__std_integration only covers std::optional<T> conversion, so the previous fix that removed its links from the score::cpp::expected / score::cpp::optional conversion tests left 12 tests in extensions_test.cpp with Description/TestType/DerivationTechnique metadata but no linkage style, violating the review checklist's 'exactly one linkage style' requirement. Author comp_req__result__futurecpp_integration to formally cover conversion to/from score::cpp::expected and to score::cpp::optional, and link the 12 orphaned tests to it via PartiallyVerifies.
Nikhil2206
requested a deployment
to
workflow-approval
September 23, 2026 10:04 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 23, 2026 10:04 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 23, 2026 10:04 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Nikhil2206
requested a deployment
to
workflow-approval
September 23, 2026 10:04 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
github-actions
Bot
added
the
documentation
Nikhil2206
marked this pull request as ready for review
September 23, 2026 10:26
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Several metadata descriptions claim coverage or guarantees that the corresponding tests and implementations do not provide.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Links all 256 score/result C++ tests to component requirements with verification metadata and adds a Future C++ integration requirement.
Changes:
- Adds requirement links and test metadata across Result tests.
- Adds coverage descriptions and derivation classifications.
- Defines a requirement for
score::cppconversions.
| File | Description |
|---|---|
score/result/result_test.cpp |
Links public Result API tests. |
score/result/error_test.cpp |
Links Error behavior tests. |
score/result/error_msg_mapping_test.cpp |
Links the Rust bridge test. |
score/result/docs/requirements/index.rst |
Adds the Future C++ integration requirement. |
score/result/details/expected/unexpected_test.cpp |
Links unexpected-type tests. |
score/result/details/expected/test_type_traits_test.cpp |
Links constructibility trait tests. |
score/result/details/expected/extensions_test.cpp |
Links conversion utility tests. |
score/result/details/expected/expected_type_trait_test.cpp |
Links expected type-trait tests. |
score/result/details/expected/expected_observer_test.cpp |
Links observer and invalid-access tests. |
score/result/details/expected/expected_monadic_operator_test.cpp |
Links monadic operation tests. |
score/result/details/expected/expected_equality_test.cpp |
Links equality tests. |
score/result/details/expected/expected_constructor_test.cpp |
Links construction tests. |
score/result/details/expected/expected_assignment_test.cpp |
Links assignment and swap tests. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+332
to
+333
| "Check that value() on an lvalue expected holding an error throws instead of returning a " | ||
| "dangling value."); |
Comment on lines
+115
to
+117
| RecordProperty("Description", | ||
| "Check that expected's move assignment is noexcept only when both the value and error types " | ||
| "are themselves nothrow move-assignable."); |
| :tags: inspected | ||
| :satisfied_by: comp__baselibs_result[version==1] | ||
|
|
||
| The Result library shall provide conversion utilities to transform an expected object into a score::cpp::expected object and vice versa, and to transform an expected's value into a score::cpp::optional<T>. The conversion to score::cpp::optional<T> shall require explicit handling of error cases before discarding error information, ensuring that errors cannot be silently ignored. |
Member
There was a problem hiding this comment.
The "expected object" in the requirement text is indeed confusing as it's an implementation detail of Result. I would not mention it in the requirement. It doesn't, however, change the essence of the requirement.
This branch was successfully deployed
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.
Summary
Link all 256 C++ tests in the
score/resultcomponent to the component requirements using the requiredRecordPropertymetadata.