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: refactor the test code use inspect func #2385

Merged

Conversation

ZYecho
Copy link
Contributor

@ZYecho ZYecho commented Oct 30, 2018

Signed-off-by: zhangyue [email protected]

Ⅰ. Describe what this PR did

  1. Use inspect func added in pr test: refactor the cli stop test #2372 to refactor part of cli test code
  2. adjust the position of some testcase which locate in wrong place or bad name.

Ⅱ. Does this pull request fix one issue?

None.

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

None.

Ⅳ. Describe how to verify it

wait for the CI.

Ⅴ. Special notes for reviews

Only refactor the part of code which use json.Unmarshal Since the inspectFilter func only return string type. When the test code need a struct type, keep it, using json.Unmarshal.

@codecov
Copy link

codecov bot commented Oct 30, 2018

Codecov Report

Merging #2385 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2385      +/-   ##
==========================================
- Coverage   68.23%   68.21%   -0.03%     
==========================================
  Files         265      265              
  Lines       18211    18211              
==========================================
- Hits        12426    12422       -4     
- Misses       4364     4369       +5     
+ Partials     1421     1420       -1
Flag Coverage Δ
#criv1alpha1test 31.45% <ø> (-0.17%) ⬇️
#criv1alpha2test 35.62% <ø> (+0.15%) ⬆️
#integrationtest 39.52% <ø> (-0.05%) ⬇️
#nodee2etest 33.08% <ø> (+0.03%) ⬆️
#unittest 25.52% <ø> (ø) ⬆️
Impacted Files Coverage Δ
daemon/mgr/system.go 70.49% <0%> (-5.74%) ⬇️
cri/v1alpha2/cri_wrapper.go 62.4% <0%> (-1.21%) ⬇️
daemon/mgr/container_utils.go 83.43% <0%> (-0.62%) ⬇️
cri/v1alpha1/cri.go 60.38% <0%> (-0.33%) ⬇️
daemon/containerio/container_io.go 75.95% <0%> (ø) ⬆️
cri/v1alpha2/cri.go 68.56% <0%> (+0.12%) ⬆️
daemon/mgr/container.go 59.39% <0%> (+0.43%) ⬆️
ctrd/container.go 59.28% <0%> (+0.95%) ⬆️

@ZYecho
Copy link
Contributor Author

ZYecho commented Oct 30, 2018

@allencloud kindly ping for review.

@allencloud
Copy link
Collaborator

Thanks a lot for your contribution. This helps a lot for the test code. @ZYecho
LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Oct 30, 2018
@allencloud allencloud merged commit 143227d into AliyunContainerService:master Oct 30, 2018
@ZYecho ZYecho deleted the replace-with-inspectFilter branch November 6, 2018 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/test kind/refactor LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants