Add `internal/runner` test #505

hlts2 · 2020-06-23T05:33:02Z

Signed-off-by: hlts2 hiroto.funakoshi.hiroto@gmail.com

Description:

I added test code for internal/runner pacakge.

Related Issue:

How Has This Been Tested?:

Environment:

Go Version: 1.14.3
Docker Version: 19.03.8
Kubernetes Version: 1.18.2
NGT Version: 1.11.5

Types of changes:

Bug fix [type/bug]
New feature [type/feature]
Add tests [type/test]
Security related changes [type/security]
Add documents [type/documentation]
Refactoring [type/refactoring]
Update dependencies [type/dependency]
Update benchmarks and performances [type/bench]
Update CI [type/ci]

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your core changes, as applicable?
Have you successfully ran tests with your changes locally?

Checklist:

I have read the CONTRIBUTING document.
I have checked open Pull Requests for the similar feature or fixes?
I have added tests and benchmarks to cover my changes.
I have ensured all new and existing tests passed.
I have commented my code, particularly in hard-to-understand areas
I have updated the documentation accordingly.

pull-assistant · 2020-06-23T05:33:04Z

Best reviewed: commit by commit

Optimal code review plan (1 warning)

feat: runner pacakge test

feat: logging error message

internal/runner/runner_test.go 79% changes removed in fix: flaky test

fix: apply suggestion

fix: flaky test

🤖 Update license headers / Format go codes and yaml files

Powered by Pull Assistant. Last update 64ee619 ... 1e25e4a. Read the comment docs.

internal/runner/option_test.go

internal/runner/runner_test.go

codecov · 2020-06-23T05:40:50Z

Codecov Report

Merging #505 into master will increase coverage by 0.54%.
The diff coverage is 80.00%.

@@            Coverage Diff            @@
##           master    #505      +/-   ##
=========================================
+ Coverage    8.31%   8.86%   +0.54%     
=========================================
  Files         402     402              
  Lines       20836   20831       -5     
=========================================
+ Hits         1733    1847     +114     
+ Misses      18853   18731     -122     
- Partials      250     253       +3

Impacted Files	Coverage Δ
internal/runner/runner.go	`90.38% <80.00%> (+90.38%)`	⬆️
internal/runner/option.go	`100.00% <0.00%> (+100.00%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85aa279...1e25e4a. Read the comment docs.

hlts2 · 2020-06-23T07:44:06Z

/rebase

vdaas-ci · 2020-06-23T07:45:00Z

[REBASE] Rebase triggered by hlts2 for branch: test/internal/runner

kevindiu · 2020-06-24T01:07:48Z

seems the test failed..

% go test -cover
2020-06-24 10:06:59	[INFO]:
build cpu info flags -> []
build time           -> Wed, 24 Jun 2020 10:06:59 JST
cgo enabled          ->
git commit           -> master
go arch              -> amd64
go os                -> darwin
go version           -> go1.14.4
ngt version          ->
server name          ->
stack trace-0        -> https://github.com/vdaas/vald/tree/master                               	github.com/vdaas/vald/internal/runner.Do
stack trace-1        -> https://github.com/vdaas/vald/tree/master                               	github.com/vdaas/vald/internal/runner.TestDo.func15
stack trace-2        -> https://github.com/golang/go/blob/go1.14.4/src/testing/testing.go#L991  	testing.tRunner
stack trace-3        -> https://github.com/golang/go/blob/go1.14.4/src/runtime/asm_amd64.s#L1373	runtime.goexit
vald version         -> v0.0.1
flag provided but not defined: -team
Usage of test:
  -c string
    	 config file path (default "/etc/server/config.yaml")
  -config string
    	 config file path (default "/etc/server/config.yaml")
  -f string
    	 config file path (default "/etc/server/config.yaml")
  -file string
    	 config file path (default "/etc/server/config.yaml")
  -v	show server version
  -ver
    	show server version
  -version
    	show server version
2020-06-24 10:06:59	[INFO]:	maxprocs: Leaving GOMAXPROCS=4: CPU quota undefined
2020-06-24 10:06:59	[INFO]:	maxprocs: Leaving GOMAXPROCS=4: CPU quota undefined
2020-06-24 10:06:59	[INFO]:	service  v1.1.2 starting...
2020-06-24 01:07:01	[WARN]:	interrupt signal received daemon will stopping soon...
2020-06-24 01:07:01	[WARN]:	daemon stopped
2020-06-24 01:07:01	[WARN]:	daemon stopped
2020-06-24 01:07:03	[ERR]:	error occured in runner.PreStop at vald: err1
2020-06-24 01:07:03	[ERR]:	error occured in runner.Stop at vald: err2
2020-06-24 01:07:03	[ERR]:	error occured in runner.PostStop at vald: err3
2020-06-24 01:07:03	[WARN]:	daemon stopped
2020-06-24 01:07:03	[ERR]:	error occured in runner.Start at vald: err1
2020-06-24 01:07:03	[ERR]:	error occured in runner.Start at vald: err2
2020-06-24 01:07:03	[ERR]:	error occured in runner.Start at vald: err1
2020-06-24 01:07:05	[WARN]:	daemon stopped
--- FAIL: TestRun (4.01s)
    --- FAIL: TestRun/returns_error_when_channel_of_run.StartFunc_contains_error (2.00s)
        runner_test.go:473: error = got error = failed to stop daemon: error:	err2	count:	1: error:	err1	count:	2, want failed to stop daemon: error:	err1	count:	2: error:	err2	count:	1
FAIL
coverage: 93.4% of statements
exit status 1
FAIL	github.com/vdaas/vald/internal/runner	6.444s

internal/runner/runner_test.go

kevindiu · 2020-06-24T04:36:55Z

For the Run test case, I think we can remove the time.Sleep() by moving the afterFunc().
What do you think?

if test.beforeFunc != nil {
				test.beforeFunc(test.args)
			}

/* remove this
			if test.afterFunc != nil {
				defer test.afterFunc(test.args)
			}
*/ 
			if test.checkFunc == nil {
				test.checkFunc = defaultCheckFunc
			}
			err := Run(test.args.ctx, test.args.run, test.args.name)

                         // here
			if test.afterFunc != nil {
 				test.afterFunc(test.args)
			}

			if err := test.checkFunc(test.want, err); err != nil {
				tt.Errorf("error = %v", err)
			}

kevindiu

LGTM

kevindiu · 2020-06-24T08:19:32Z

/rebase

vdaas-ci · 2020-06-24T08:20:08Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/runner

hlts2 · 2020-06-25T01:42:17Z

/rebase

vdaas-ci · 2020-06-25T01:42:49Z

[REBASE] Rebase triggered by hlts2 for branch: test/internal/runner

hlts2 · 2020-06-29T01:11:34Z

/rebase

vdaas-ci · 2020-06-29T01:12:10Z

[REBASE] Rebase triggered by hlts2 for branch: test/internal/runner

vankichi

LGTM

vankichi · 2020-06-29T03:07:28Z

/rebase
/format
/approve

vdaas-ci · 2020-06-29T03:08:10Z

[REBASE] Rebase triggered by vankichi for branch: test/internal/runner

vdaas-ci · 2020-06-29T03:09:09Z

[FORMAT] Updating license headers and formatting go codes triggered by vankichi.

vdaas-ci

[APPROVED] This PR is approved by vankichi.

vankichi · 2020-06-29T03:20:26Z

/rebase
/approve

vdaas-ci · 2020-06-29T03:21:01Z

[REBASE] Rebase triggered by vankichi for branch: test/internal/runner

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

Signed-off-by: vdaas-ci <ci@vdaas.org>

vdaas-ci

[APPROVED] This PR is approved by vankichi.

hlts2 requested a review from kevindiu June 23, 2020 05:33

hlts2 self-assigned this Jun 23, 2020

github-actions bot added area/internal team/set SET team size/XXL type/test Adding tests priority/low labels Jun 23, 2020

github-actions bot reviewed Jun 23, 2020

View reviewed changes

internal/runner/option_test.go Show resolved Hide resolved

internal/runner/runner_test.go Show resolved Hide resolved

vdaas-ci force-pushed the test/internal/runner branch from 119a92d to e4e2aa4 Compare June 23, 2020 07:45

kevindiu reviewed Jun 24, 2020

View reviewed changes

internal/runner/runner_test.go Outdated Show resolved Hide resolved

github-actions bot reviewed Jun 24, 2020

View reviewed changes

internal/runner/runner_test.go Show resolved Hide resolved

kevindiu previously approved these changes Jun 24, 2020

View reviewed changes

hlts2 requested a review from vankichi June 24, 2020 06:59

vdaas-ci dismissed kevindiu’s stale review via 6106d46 June 24, 2020 08:20

vdaas-ci force-pushed the test/internal/runner branch from e71b8ad to 6106d46 Compare June 24, 2020 08:20

kevindiu previously approved these changes Jun 25, 2020

View reviewed changes

vdaas-ci dismissed kevindiu’s stale review via c7ead76 June 29, 2020 01:12

vdaas-ci force-pushed the test/internal/runner branch from 6106d46 to c7ead76 Compare June 29, 2020 01:12

kevindiu previously approved these changes Jun 29, 2020

View reviewed changes

vankichi previously approved these changes Jun 29, 2020

View reviewed changes

vdaas-ci dismissed stale reviews from vankichi and kevindiu via eccde19 June 29, 2020 03:10

vdaas-ci previously approved these changes Jun 29, 2020

View reviewed changes

hlts2 and others added 5 commits June 29, 2020 03:21

feat: runner pacakge test

64ee619

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

feat: logging error message

3685d93

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: apply suggestion

994cd1d

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: flaky test

fc934fe

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

🤖 Update license headers / Format go codes and yaml files

1e25e4a

Signed-off-by: vdaas-ci <ci@vdaas.org>

vdaas-ci dismissed their stale review via 1e25e4a June 29, 2020 03:21

vdaas-ci force-pushed the test/internal/runner branch from eccde19 to 1e25e4a Compare June 29, 2020 03:21

vdaas-ci approved these changes Jun 29, 2020

View reviewed changes

vankichi merged commit 9e1f1e8 into master Jun 29, 2020

vankichi deleted the test/internal/runner branch June 29, 2020 03:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `internal/runner` test #505

Add `internal/runner` test #505

hlts2 commented Jun 23, 2020 •

edited

Loading

pull-assistant bot commented Jun 23, 2020 •

edited

Loading

codecov bot commented Jun 23, 2020 •

edited

Loading

hlts2 commented Jun 23, 2020

vdaas-ci commented Jun 23, 2020

kevindiu commented Jun 24, 2020

kevindiu commented Jun 24, 2020

kevindiu left a comment

kevindiu commented Jun 24, 2020

vdaas-ci commented Jun 24, 2020

hlts2 commented Jun 25, 2020

vdaas-ci commented Jun 25, 2020

hlts2 commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vankichi left a comment

vankichi commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vdaas-ci left a comment

vankichi commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vdaas-ci left a comment

Add internal/runner test #505

Add internal/runner test #505

Conversation

hlts2 commented Jun 23, 2020 • edited Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

pull-assistant bot commented Jun 23, 2020 • edited Loading

codecov bot commented Jun 23, 2020 • edited Loading

Codecov Report

hlts2 commented Jun 23, 2020

vdaas-ci commented Jun 23, 2020

kevindiu commented Jun 24, 2020

kevindiu commented Jun 24, 2020

kevindiu left a comment

Choose a reason for hiding this comment

kevindiu commented Jun 24, 2020

vdaas-ci commented Jun 24, 2020

hlts2 commented Jun 25, 2020

vdaas-ci commented Jun 25, 2020

hlts2 commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vankichi left a comment

Choose a reason for hiding this comment

vankichi commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

vankichi commented Jun 29, 2020

vdaas-ci commented Jun 29, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

Add `internal/runner` test #505

Add `internal/runner` test #505

hlts2 commented Jun 23, 2020 •

edited

Loading

pull-assistant bot commented Jun 23, 2020 •

edited

Loading

codecov bot commented Jun 23, 2020 •

edited

Loading