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 tests for label and config file in daemon #1087

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

Letty5411
Copy link
Contributor

Signed-off-by: letty letty.ll@alibaba-inc.com

Ⅰ. Describe what this PR did

Add deamon tests for :

  1. label
  2. configure file

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@Letty5411 Letty5411 force-pushed the 0410 branch 2 times, most recently from 9a9a113 to 56e2f40 Compare April 10, 2018 09:03
@pouchrobot pouchrobot added size/XL and removed size/L labels Apr 10, 2018
@Letty5411 Letty5411 force-pushed the 0410 branch 2 times, most recently from f3f2925 to b853d2b Compare April 10, 2018 10:43
@Letty5411 Letty5411 changed the title [WIP] test: add tests for label and config file in daemon test: add tests for label and config file in daemon Apr 10, 2018
@codecov-io
Copy link

codecov-io commented Apr 10, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1087      +/-   ##
==========================================
+ Coverage   16.29%   16.47%   +0.17%     
==========================================
  Files         161      161              
  Lines        8851     8870      +19     
==========================================
+ Hits         1442     1461      +19     
- Misses       7303     7306       +3     
+ Partials      106      103       -3
Impacted Files Coverage Δ
client/client.go 25% <0%> (-3.58%) ⬇️
daemon/mgr/cri_stream.go 0% <0%> (ø) ⬆️
daemon/mgr/container_types.go 15.38% <0%> (ø) ⬆️
cli/cli.go 0% <0%> (ø) ⬆️
cli/network.go 0% <0%> (ø) ⬆️
daemon/mgr/cri.go 0% <0%> (ø) ⬆️
daemon/mgr/container.go 5.11% <0%> (+0.11%) ⬆️
cli/volume.go 8.33% <0%> (+0.13%) ⬆️
pkg/utils/utils.go 82.06% <0%> (+3.91%) ⬆️
pkg/kernel/kernel.go 100% <0%> (+27.27%) ⬆️

@@ -148,7 +152,17 @@ func (d *Config) StartDaemon() error {
if util.WaitTimeout(time.Duration(d.timeout)*time.Second, d.IsDaemonUp) == false {
if d.Debug == true {
d.DumpLog()

fmt.Printf("\n")
fmt.Printf("Failed to launch pouchd:%v\n", d.Args)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could use fmt.Printf("\nFailed to launch pouchd:%v\n", d.Args)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good

c.Assert(err, check.IsNil)

// verify listen to tcp works
command.PouchRun("--host", addr, "version").Assert(c, icmd.Success)

dcfg.KillDaemon()
defer dcfg.KillDaemon()
Copy link
Contributor

Choose a reason for hiding this comment

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

why KillDaemon() twice?

c.Assert(err, check.NotNil)
}

// TestDaemonConfigFileUnknownFlag tests start daemon with unknow flags in configure file.
Copy link
Contributor

Choose a reason for hiding this comment

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

%s/unknow/unknown

@Letty5411 Letty5411 force-pushed the 0410 branch 2 times, most recently from fa51637 to a13d235 Compare April 10, 2018 14:13
Signed-off-by: letty <letty.ll@alibaba-inc.com>
@Letty5411
Copy link
Contributor Author

@HusterWan code is ready, PTAL.

@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Apr 11, 2018
@allencloud allencloud merged commit c67f0c8 into AliyunContainerService:master Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/test 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.

5 participants