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 node e2e test in CI #2198

Merged
merged 1 commit into from
Sep 10, 2018

Conversation

starnop
Copy link
Contributor

@starnop starnop commented Sep 5, 2018

Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

Currently, we have added the unit tests related to CRI and the tests of CRI-tools to CI, but this is not reassuring enough, so we add the node e2e test to further guarantees the correctness of the project.

Ⅱ. 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

Ⅴ. Special notes for reviews

@starnop starnop force-pushed the cri-e2e-ci branch 22 times, most recently from 06d776f to 788b19a Compare September 6, 2018 03:38
@codecov-io
Copy link

codecov-io commented Sep 6, 2018

Codecov Report

Merging #2198 into master will increase coverage by 2.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2198     +/-   ##
=========================================
+ Coverage    64.9%   67.01%   +2.1%     
=========================================
  Files         208      208             
  Lines       16722    17064    +342     
=========================================
+ Hits        10853    11435    +582     
+ Misses       4519     4281    -238     
+ Partials     1350     1348      -2
Flag Coverage Δ
#criv1alpha1test 32.96% <ø> (ø) ⬆️
#criv1alpha2test 33.5% <ø> (-0.14%) ⬇️
#integrationtest 39.93% <ø> (-0.07%) ⬇️
#nodee2etest 34.99% <ø> (?)
#unittest 23.84% <ø> (ø) ⬆️
Impacted Files Coverage Δ
cri/v1alpha2/cri_types.go 100% <0%> (ø) ⬆️
daemon/mgr/container.go 57.17% <0%> (+0.4%) ⬆️
daemon/mgr/system.go 79.5% <0%> (+1.63%) ⬆️
ctrd/image.go 78.94% <0%> (+1.75%) ⬆️
cri/stream/remotecommand/httpstream.go 46.63% <0%> (+2.59%) ⬆️
ctrd/watch.go 83.33% <0%> (+3.03%) ⬆️
daemon/logger/jsonfile/encode.go 80.45% <0%> (+3.44%) ⬆️
daemon/containerio/cri_log_file.go 88.23% <0%> (+3.92%) ⬆️
daemon/mgr/spec_mount.go 84.11% <0%> (+4.67%) ⬆️
ctrd/container.go 60.52% <0%> (+6.69%) ⬆️
... and 8 more

@starnop starnop changed the title [WIP] test: add node e2e test in CI test: add node e2e test in CI Sep 6, 2018
@allencloud
Copy link
Collaborator

This is really good. Thanks a lot for your work. @starnop

@YaoZengzeng
Copy link
Contributor

👍

fi

command -v ginkgo > /dev/null
chmod +x hack/install/install_ginkgo.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

we can chmod hack/install/install_ginkgo.sh before push

EOF'

echo
chmod +x hack/install/install_cni.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

we can chmod file before push

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. 👍

@starnop starnop force-pushed the cri-e2e-ci branch 2 times, most recently from 6855606 to 92a1a5b Compare September 7, 2018 03:44
Makefile Outdated
cri-e2e-test: ## run cri-e2e-test
@echo $@
@mkdir -p coverage
@chmod +x ./hack/testing/run_daemon_cri_e2e.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

chmod before push

@starnop starnop force-pushed the cri-e2e-ci branch 7 times, most recently from 5d2ec79 to 6d0a86d Compare September 10, 2018 03:29
Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM
ping @YaoZengzeng

@allencloud
Copy link
Collaborator

Thanks for your review, I think maybe we could kick ball first, and see what will go on in the next CI. @fuweid
But if @YaoZengzeng would like to review this in the future, it could not be better.
Thus, I will merge this.

@allencloud allencloud merged commit c544489 into AliyunContainerService:master Sep 10, 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