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.

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

test:add container resize and restart test #1090

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

Dewey-Ding
Copy link
Contributor

Signed-off-by: Dewey-Ding deweyding@gmail.com

Ⅰ. Describe what this PR did

add container resize and restart test

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

Codecov Report

Merging #1090 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1090      +/-   ##
==========================================
- Coverage   16.47%   16.39%   -0.09%     
==========================================
  Files         161      163       +2     
  Lines        8921     8813     -108     
==========================================
- Hits         1470     1445      -25     
+ Misses       7347     7264      -83     
  Partials      104      104
Impacted Files Coverage Δ
client/container.go 0% <ø> (ø) ⬆️
client/container_restart.go 100% <100%> (ø)
client/container_resize.go 100% <100%> (ø)
pkg/kernel/kernel.go 75% <0%> (-5%) ⬇️
pkg/utils/utils.go 80.58% <0%> (-1.49%) ⬇️
cli/network.go 0% <0%> (ø) ⬆️
cli/cli.go 0% <0%> (ø) ⬆️
cli/volume.go 8.33% <0%> (+0.13%) ⬆️
... and 1 more

@allencloud
Copy link
Collaborator

Could you help to review this? @ZouRui89
Thanks a lot.


httpClient := newMockClient(func(req *http.Request) (*http.Response, error) {
if !strings.HasPrefix(req.URL.Path, expectedURL) {
return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the code style rules of pouch, No matter log or error, first letter of the message must be lower-case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am following the example write by allencloud in issue #966 .Is there special?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid that @zhuangqh has made a point and we should change all the case problem in mock test. And I have noticed that the same problem exists in other part of Pouch. WDYT? @allencloud

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think what @zhuangqh mentioned is reasonable. Could someone fix this totally in a single pull request. @ZouRui89

Copy link
Contributor

Choose a reason for hiding this comment

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

@Dewey-Ding Could you fix the case problem? Thx a lot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe i can do it in this pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: Dewey-Ding <deweyding@gmail.com>
@ZouRui89
Copy link
Contributor

LGTM

@ZouRui89 ZouRui89 merged commit 1d1bec1 into AliyunContainerService:master Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants