Skip to content

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented Aug 4, 2025

Description

Fix #1299

Test

No need for new tests. Would be to add a test here, but we are already checking for the error, no need to check that we aren't overwriting the incorrect file:
https://github.com/kubewarden/kwctl/blob/main/src/scaffold/admission_request.rs#L771

Additional Information

Tradeoff

Potential improvement

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad requested a review from a team as a code owner August 4, 2025 11:29
@github-project-automation github-project-automation bot moved this to Pending review in Kubewarden Aug 4, 2025
@viccuad viccuad removed this from Kubewarden Aug 4, 2025
Use `Path` instead of `PathBuf` to avoid useless `clone()` invocations.

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Copy link

codecov bot commented Aug 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.46%. Comparing base (9b74b44) to head (af0963c).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1300      +/-   ##
==========================================
+ Coverage   86.69%   87.46%   +0.77%     
==========================================
  Files          34       34              
  Lines        4939     4932       -7     
==========================================
+ Hits         4282     4314      +32     
+ Misses        657      618      -39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

viccuad added a commit to viccuad/kwctl that referenced this pull request Aug 4, 2025
This test case doubles as checking for not overwriting the `--object` file
see kubewarden#1300

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@flavio
Copy link
Member

flavio commented Aug 4, 2025

wait a second, I'm coming up with a unit test

Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

I've added a e2e that can spot the regression

flavio pushed a commit to viccuad/kwctl that referenced this pull request Aug 5, 2025
This test case doubles as checking for not overwriting the `--object` file
see kubewarden#1300

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@flavio
Copy link
Member

flavio commented Aug 5, 2025

I did a forced push because I forgot to add the fixture file used by the tests 🤦

@flavio
Copy link
Member

flavio commented Aug 5, 2025

Warning, GHA has not been triggered by my push...

Ignore me, I pushed to the wrong branch. Tests are running now

Avoid regressions of kubewarden#1299

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
@flavio flavio force-pushed the fix/cache-resource-catalog branch from d3c8e29 to af0963c Compare August 5, 2025 07:23
flavio pushed a commit to viccuad/kwctl that referenced this pull request Aug 5, 2025
This test case doubles as checking for not overwriting the `--object` file
see kubewarden#1300

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad merged commit 6e4108d into kubewarden:main Aug 5, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kwctl scaffold admission-request --object test.yaml overwrites the file contents if no resource catalog cache is present
3 participants