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

Unit tests for kubeadm upgrade diff #826

Closed
5 tasks
liztio opened this issue May 16, 2018 · 1 comment · Fixed by kubernetes/kubernetes#63991
Closed
5 tasks

Unit tests for kubeadm upgrade diff #826

liztio opened this issue May 16, 2018 · 1 comment · Fixed by kubernetes/kubernetes#63991
Assignees
@liztio
Labels
area/test priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone
v1.11

Comments

@liztio
Copy link

liztio commented May 16, 2018

Some suggestions from @neolit123:

  • non existing config path in flags.parent.cfgPath
  • flags.newK8sVersionStr == "", but no version arg is provided
  • bad version format in flags.newK8sVersionStr to make ParseSemantic() fail
  • make kubeadmutil.MarshalToYaml fail somehow.
  • fail ioutil.ReadFile(path) by providing bad 'path'
@liztio liztio added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. area/test labels May 16, 2018
@liztio liztio added this to the v1.11 milestone May 16, 2018
@liztio liztio self-assigned this May 16, 2018
@liztio liztio mentioned this issue May 16, 2018
Merged
@neolit123 neolit123 mentioned this issue May 17, 2018
Merged
@neolit123
Copy link
Member

^ submitted PR for the UTs.

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue May 22, 2018
Merge pull request #63991 from neolit123/test-upgrade-diff
10b8665 
Automatic merge from submit-queue (batch tested with PRs 62025, 63851, 64077, 63967, 63991). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add unit tests to `kubeadm upgrade diff` and small improvements

**What this PR does / why we need it**:

has a couple of commits:

I.
```
1) Store the io.Writer and pass it to sub-commands in upgrade.go
2) Check if the manifest path is an empty string in diff.go:runDiff()
3) Use the io.Writer that upgrade.go defines instead of writing to
os.Stdout directly.
```
II.
```
Add the file diff_test.go, which has a single test:
  TestRunDiff

The test covers most error cases for the runDiff() function,
and also performs a valid diff.
```


**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes kubernetes/kubeadm#826

**Special notes for your reviewer**:
@liztio @luxas 

**Release note**:

```release-note
NONE
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@liztio liztio

Labels
area/test priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Milestone
v1.11
Development

Successfully merging a pull request may close this issue.

Add unit tests to kubeadm upgrade diff and small improvements neolit123/kubernetes
2 participants
@neolit123 @liztio