Skip to content

Tests: Replace Update() calls with Patch()#401

Open
alromeros wants to merge 2 commits into
kubevirt:mainfrom
alromeros:update-to-patch
Open

Tests: Replace Update() calls with Patch()#401
alromeros wants to merge 2 commits into
kubevirt:mainfrom
alromeros:update-to-patch

Conversation

@alromeros

@alromeros alromeros commented Jan 22, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

Some tests have been failing due to update conflicts. Using Patch() instead of Update() avoids these races and follows the pattern already adopted in other kubevirt repositories.

This PR:

  • Adds a helper for building JSON patches.
  • Refactors affected tests to use Patch() instead of Update().

Special notes for your reviewer:

Assisted-By: Claude noreply@anthropic.com

Release note:

NONE

@kubevirt-bot kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jan 22, 2026
@kubevirt-bot

kubevirt-bot commented Jan 22, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mhenriks for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@alromeros

alromeros commented Jan 22, 2026

Copy link
Copy Markdown
Member Author

/test pull-kvp-functional-test

2 similar comments
@alromeros

alromeros commented Jan 23, 2026

Copy link
Copy Markdown
Member Author

/test pull-kvp-functional-test

@alromeros

alromeros commented Jan 26, 2026

Copy link
Copy Markdown
Member Author

/test pull-kvp-functional-test

@alromeros alromeros force-pushed the update-to-patch branch 2 times, most recently from 385217a to fea61e9 Compare January 27, 2026 12:20
@alromeros
Tests: Add JSON patch builder package
22fe00c 
This commit includes a new patch package to easily build JSON patches. This will allow easy refactoring of tests using Update, which is more error prone.

Signed-off-by: Alvaro Romero <alromero@redhat.com>

Assisted-by: Claude <noreply@anthropic.com>
@alromeros

alromeros commented Feb 4, 2026

Copy link
Copy Markdown
Member Author

/test pull-kvp-functional-test

ShellyKa13
ShellyKa13 reviewed Feb 23, 2026
View reviewed changes
Comment thread tests/resource_filtering_test.go Outdated
}

retryOnceOnErr(updateDataVolume(f.KvClient, f.Namespace.Name, name, updateFunc)).Should(BeNil())
retryOnceOnErr(func() error {

@ShellyKa13 ShellyKa13 Feb 23, 2026

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.

do we still need the retryOnceOnErr for patch?

@alromeros alromeros Feb 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think we do, good catch!

Comment thread tests/resource_filtering_test.go Outdated
return err
}
patchData, err := patch.New(
patch.WithAddLabel("velero.io/exclude-from-backup", "true", dv.Labels),

@ShellyKa13 ShellyKa13 Feb 23, 2026

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.

Can we use this velero const for this label: https://github.com/vmware-tanzu/velero/blob/ba5e7681ff60641cd6427619181bf7991bb43c7e/pkg/apis/velero/v1/labels_annotations.go#L103 ?

@alromeros
Tests: Use Patch instead of Update
fc6385c 
This commit refactors tests to use Patch client calls instead of Updates.

Signed-off-by: Alvaro Romero <alromero@redhat.com>

Assisted-by: Claude <noreply@anthropic.com>
@alromeros

alromeros commented Mar 16, 2026

Copy link
Copy Markdown
Member Author

/test pull-kvp-functional-test

weshayutin
weshayutin reviewed Mar 17, 2026
View reviewed changes

@weshayutin weshayutin 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.

looks good.. LET'S GO @aglitke :)

@weshayutin

weshayutin commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

/retest

@weshayutin

weshayutin commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

/test all

@alromeros

alromeros commented Mar 25, 2026

Copy link
Copy Markdown
Member Author

/test pull-kvp-functional-test

@alromeros

alromeros commented Mar 25, 2026

Copy link
Copy Markdown
Member Author

Trying to figure out where these flakes come from.

@alromeros alromeros requested a review from ShellyKa13 April 21, 2026 08:42
@alromeros

alromeros commented Apr 21, 2026

Copy link
Copy Markdown
Member Author

/cc @shubham-pampattiwar

@kubevirt-bot

kubevirt-bot commented Jun 5, 2026

Copy link
Copy Markdown

There has been no activity on this PR for 45 days.
To protect limited CI resources, it has been automatically labelled 'stale'.
This PR will automatically rot after an additional 14 days of inactivity, and will be closed shortly after that.

What you can do:

  • If the PR is waiting on you to respond to a question or feedback and/or update the PR, please do so.
  • You can mark the PR as fresh and remove the label with the following command: /remove-lifecycle stale
  • If this PR is safe to close now, please help the project by closing it with: /close
  • If you need attention on this PR from a reviewer, you can raise it on the agenda of the relevant SIG meeting or KubeVirt Community meeting, or ping the kubevirt-dev slack channel.

/lifecycle stale

@kubevirt-bot kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 5, 2026
@alromeros

alromeros commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

/remove-lifecycle stale

@kubevirt-bot kubevirt-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sseago sseago Awaiting requested review from sseago
@ShellyKa13 ShellyKa13 Awaiting requested review from ShellyKa13
@shubham-pampattiwar shubham-pampattiwar Awaiting requested review from shubham-pampattiwar
1 more reviewer
@weshayutin weshayutin weshayutin left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alromeros @kubevirt-bot @weshayutin @ShellyKa13