Skip to content

fix: AddComponentVersion was writing to the same file#2013

Merged
Skarlso merged 1 commit into
open-component-model:mainfrom
Skarlso:fix-flaking-plugin-test
Mar 18, 2026
Merged

fix: AddComponentVersion was writing to the same file#2013
Skarlso merged 1 commit into
open-component-model:mainfrom
Skarlso:fix-flaking-plugin-test

Conversation

@Skarlso

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

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Fixes https://github.com/open-component-model/open-component-model/actions/runs/23254126246/attempts/1

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

Release Notes

  • Tests
    • Improved reliability of integration tests with enhanced error handling for file operations.

Note: This release contains internal testing improvements with no user-facing changes.

@Skarlso
fix: AddComponentVersion was writing to the same file
506ca71 
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso Skarlso requested a review from a team as a code owner March 18, 2026 16:25
@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: 7072c424-455c-4004-95ed-f6920618635c

📥 Commits

Reviewing files that changed from the base of the PR and between 14e98e9 and 506ca71.

📒 Files selected for processing (1)
  • cli/integration/plugin_registry_list_integration_test.go
📝 Walkthrough

Walkthrough

A test file was modified to replace manual temporary file path construction with the os.CreateTemp function, including proper error handling for file creation, writing, and closure operations.

Changes

Cohort / File(s) Summary
Test Utility Refactoring
cli/integration/plugin_registry_list_integration_test.go		 Replaced filepath.Join(os.TempDir(), "constructor.yaml") with os.CreateTemp("", "constructor-*.yaml"), adding error handling for file creation, content writing via WriteString, and explicit file closure. File path now derived from temporary file's Name() method.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A temp file once lived in confusion,
With paths built by hand—what an illusion!
Now CreateTemp takes the lead,
With errors handled, just what we need,
The test hops along, clean and bright! ✨

🚥 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 title accurately describes the main change: fixing a bug where AddComponentVersion was writing to the same file, which is addressed by switching to proper temporary file creation with os.CreateTemp instead of manual path construction.

✏️ 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.

matthiasbruns
matthiasbruns reviewed Mar 18, 2026
View reviewed changes
Comment thread cli/integration/plugin_registry_list_integration_test.go
@Skarlso Skarlso merged commit 849b6e1 into open-component-model:main Mar 18, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthiasbruns matthiasbruns matthiasbruns approved these changes

@frewilhelm frewilhelm frewilhelm approved these changes

Assignees

No one assigned

Labels

kind/bugfix Bug size/s Small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Skarlso @matthiasbruns @frewilhelm