Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

unittests: Fixes unit tests for Windows #111534

Merged
merged 1 commit into from Oct 24, 2022
Merged

unittests: Fixes unit tests for Windows #111534

merged 1 commit into from Oct 24, 2022

Conversation

claudiubelu
Copy link
Contributor

@claudiubelu claudiubelu commented Jul 29, 2022 •
edited

What type of PR is this?

/kind failing-test

/sig testing
/sig windows
/priority important-soon

What this PR does / why we need it:

Currently, there are some unit tests that are failing on Windows due to various reasons:

  • Windows file permissions do not work the same way as the Linux ones.
  • cp does not exist on Windows, and xcopy should be used instead.
  • Get-Item does not work for hidden files / folders like AppData, but works if given the -Force flag.

Which issue(s) this PR fixes:

Related: #51540

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/windows Categorizes an issue or PR as relevant to SIG Windows. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 29, 2022
@k8s-ci-robot k8s-ci-robot added area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Jul 29, 2022
neolit123
neolit123 reviewed Jul 29, 2022
View reviewed changes
Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

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

/approve

for cmd/kubeadm. thanks

@claudiubelu
Copy link
Contributor Author

This PR has been included in this PR #110981, in which we've ran the pull-ci-kubernetes-unit-windows job, in which we can see that these tests now pass.

@claudiubelu
Copy link
Contributor Author

/test pull-kubernetes-e2e-capz-windows-containerd

@marosset marosset added this to In Progress (v1.25) in SIG-Windows Aug 1, 2022
@claudiubelu
Copy link
Contributor Author

/assign @jingxu97

@RA489
Copy link

RA489 commented Aug 12, 2022

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 12, 2022
@claudiubelu
Copy link
Contributor Author

/milestone v1.26

@k8s-ci-robot k8s-ci-robot added this to the v1.26 milestone Aug 25, 2022
@claudiubelu claudiubelu moved this from In Progress (v1.25) to In Review (v1.26) in SIG-Windows Aug 25, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 25, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 26, 2022
@claudiubelu claudiubelu force-pushed the unittests-7 branch 2 times, most recently from 381a52a to 08332e7 Compare August 26, 2022 10:27
@claudiubelu
unittests: Fixes unit tests for Windows
49ef270 
Currently, there are some unit tests that are failing on Windows due to
various reasons:

- Windows file permissions do not work the same way as the Linux ones.
- cp does not exist on Windows, and xcopy should be used instead.
- Get-Item does not work for hidden files / folders like AppData, but
  works if given the -Force flag.
@claudiubelu
Copy link
Contributor Author

/test pull-kubernetes-e2e-capz-windows-containerd

@claudiubelu
Copy link
Contributor Author

/assign @jingxu97

@marosset marosset removed this from In Review (v1.26) in SIG-Windows Sep 21, 2022
@pacoxu pacoxu mentioned this pull request Sep 26, 2022
Merged
@salaxander
Copy link

Hey @claudiubelu - 1.26 bug triage team checking in! Just wanted to see if we're on track to get this in for the 1.26 release?

Thanks!

@claudiubelu
Copy link
Contributor Author

Hey @claudiubelu - 1.26 bug triage team checking in! Just wanted to see if we're on track to get this in for the 1.26 release?

Thanks!

Yes, it is. It is half approved at this point, we need pkg/volume/util/subpath owners to approve.

@claudiubelu
Copy link
Contributor Author

/assign @jsafrane

@jsafrane
Copy link
Member

/approve
pkg/volume changes
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: claudiubelu, jsafrane, neolit123

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

The pull request process is described here

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2022
@k8s-ci-robot k8s-ci-robot merged commit 38c659e into kubernetes:master Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123 neolit123 left review comments

@freehan freehan Awaiting requested review from freehan

Assignees

@jsafrane jsafrane

@jingxu97 jingxu97

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
[sig-release] Bug Triage
Archived in project
[sig-windows] PR tracking
Status: Done
Milestone
v1.26
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@claudiubelu @RA489 @salaxander @jsafrane @k8s-ci-robot @neolit123 @jingxu97