Skip to content

chore: improve error handling and assertions in tests#2009

Merged
jakobmoellerdev merged 4 commits into
open-component-model:mainfrom
fabianburth:chore/compref-tests
Mar 18, 2026
Merged

chore: improve error handling and assertions in tests#2009
jakobmoellerdev merged 4 commits into
open-component-model:mainfrom
fabianburth:chore/compref-tests

Conversation

@fabianburth

@fabianburth fabianburth commented Mar 18, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Follow up for a coderabbitai comment on a previous PR

Which issue(s) this PR fixes

Testing

How to test the changes
Verification
  • I have tested the changes locally by running ocm

Summary by CodeRabbit

  • Tests
    • Improved test error handling to stop and report immediately when parsing fails.
    • Added explicit non-null validation for parsed results on success.
    • Strengthened serialization checks by comparing entire objects for exact equality rather than partial string matching.

@fabianburth
chore: improve error handling and assertions in tests
a9f6ac9 
Refactor test cases to handle errors more gracefully and ensure
expected values are correctly asserted.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@fabianburth fabianburth requested a review from a team as a code owner March 18, 2026 08:53
@github-actions github-actions Bot added kind/chore chore, maintenance, etc. size/xs Extra small labels Mar 18, 2026
@coderabbitai

coderabbitai Bot commented Mar 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0dcaf89f-5b96-43c7-9453-5a8174b4c9a2

📥 Commits

Reviewing files that changed from the base of the PR and between a9f6ac9 and 39f4917.

📒 Files selected for processing (1)
  • bindings/go/oci/compref/compref_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • bindings/go/oci/compref/compref_test.go
📝 Walkthrough

Walkthrough

Updates test assertions in bindings/go/oci/compref/compref_test.go: refactors error handling to call the test-specific error checker with early return, asserts non-nil parsed objects on success, and replaces partial string containment checks with full struct equality assertions.

Changes

Cohort / File(s) Summary
Test Assertion Updates
bindings/go/oci/compref/compref_test.go		 Refactored test error handling to call tc.err(t, err) and return on error; added explicit non-nil assertion for parsed results; replaced Contains-based string check with strict equality comparison between expected and parsed structs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hopped through tests with care and cheer,
Replaced loose matches so truths appear,
Errors handled, early flight,
Structs now match—everything's right,
A tidy test burrow, cozy and clear.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'chore: improve error handling and assertions in tests' directly aligns with the primary changes in the changeset, which involve adjusting error handling and replacing partial assertions with full equality checks in test code.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed Mar 18, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
bindings/go/oci/compref/compref_test.go (1)

344-346: Minor: Error message is misleading.

The assertion now compares the parsed struct directly with the expected struct (testing parsing correctness), but the error message still says "did not serialize properly". Consider updating the message to reflect what's being tested.

📝 Suggested message fix 
 if tc.expected != nil {
-    r.EqualValues(tc.expected, parsed, "input %q did not serialize properly", tc.input)
+    r.EqualValues(tc.expected, parsed, "input %q did not parse correctly", tc.input)
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bindings/go/oci/compref/compref_test.go` around lines 344 - 346, Update the
assertion message to accurately describe what's being tested: replace the
misleading "did not serialize properly" text used in the r.EqualValues assertion
that compares tc.expected and parsed (in the test that uses variables tc.input,
tc.expected, parsed) with a message like "did not parse properly" or "parsed
value did not match expected for input %q" so failures reflect
parsing/deserialization issues rather than serialization.
🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@bindings/go/oci/compref/compref_test.go`:
- Around line 344-346: Update the assertion message to accurately describe
what's being tested: replace the misleading "did not serialize properly" text
used in the r.EqualValues assertion that compares tc.expected and parsed (in the
test that uses variables tc.input, tc.expected, parsed) with a message like "did
not parse properly" or "parsed value did not match expected for input %q" so
failures reflect parsing/deserialization issues rather than serialization.
ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 60360a36-a360-49bf-ad1c-d47e11718999

📥 Commits

Reviewing files that changed from the base of the PR and between 651cc58 and a9f6ac9.

📒 Files selected for processing (1)
  • bindings/go/oci/compref/compref_test.go

Skarlso
Skarlso reviewed Mar 18, 2026
View reviewed changes
Comment thread bindings/go/oci/compref/compref_test.go
@fabianburth
test(compref): enhance assertions in component reference tests
39f4917 
Added additional checks to ensure parsed values are not nil and updated
equality assertions for better clarity in test failures.

Signed-off-by: Fabian Burth <fabian.burth@sap.com>
@github-actions github-actions Bot added the size/s Small label Mar 18, 2026
@jakobmoellerdev jakobmoellerdev enabled auto-merge (squash) March 18, 2026 18:23
@jakobmoellerdev jakobmoellerdev merged commit ceb9a1b into open-component-model:main Mar 18, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Skarlso Skarlso Skarlso left review comments

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@matthiasbruns matthiasbruns matthiasbruns approved these changes

@jakobmoellerdev jakobmoellerdev jakobmoellerdev approved these changes

Assignees

No one assigned

Labels

kind/chore chore, maintenance, etc. size/s Small size/xs Extra small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fabianburth @Skarlso @matthiasbruns @jakobmoellerdev