Fix failed test case of internal/safety package #464

kevindiu · 2020-06-10T07:29:02Z

Description:

This PR fix the failed test case in internal/safety package.

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-10T07:29:12Z

Best reviewed: commit by commit

Optimal code review plan (2 warnings, 2 commits squashed)

fix test case

fix

genereate missing test code

add coding doc ... fix doc

Squashed 2 commits:

add coding doc

fix doc

...s/contributing/coding-style.md 62% changes removed in fix formatting

Apply suggestions from code review

...s/contributing/coding-style.md 71% changes removed in add title

Apply suggestions from code review

fix formatting

add title

Powered by Pull Assistant. Last update bfa2efd ... 60bb2fb. Read the comment docs.

kevindiu · 2020-06-10T07:30:48Z

/format
/rebase

vdaas-ci · 2020-06-10T07:31:36Z

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

vdaas-ci · 2020-06-10T07:32:36Z

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

vdaas-ci · 2020-06-10T07:33:26Z

[FORMAT] Failed to format.

codecov · 2020-06-10T07:36:02Z

Codecov Report

Merging #464 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #464   +/-   ##
======================================
  Coverage    7.83%   7.83%           
======================================
  Files         396     396           
  Lines       20322   20322           
======================================
  Hits         1593    1593           
  Misses      18492   18492           
  Partials      237     237

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 972e943...60bb2fb. Read the comment docs.

hlts2

Thank you for your PR:bow:
LGTM

vankichi · 2020-06-10T10:35:01Z

/rebase

vdaas-ci · 2020-06-10T10:35:34Z

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

vankichi

LGTM

vankichi · 2020-06-11T01:13:22Z

/approve
/rebase
/approve

vdaas-ci · 2020-06-11T01:14:00Z

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

vdaas-ci

[APPROVED] This PR is approved by vankichi.

vankichi · 2020-06-11T05:38:05Z

/rebase

vdaas-ci · 2020-06-11T05:43:03Z

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

docs/contributing/coding-style.md

vankichi

@kevindiu I left some suggestions. please check it.
I suggest you one sentence in a line would be better. (We can check difference easy if there are some changes in another PRs. )

docs/contributing/coding-style.md

github-actions · 2020-06-15T05:21:11Z

internal/safety/safety_test.go

@@ -24,65 +24,138 @@ import (
 	"go.uber.org/goleak"
 )

+var (
+	// Goroutine leak is detected by `fastime`, but it should be ignored in the test because it is an external package.
+	goleakIgnoreOptions = []goleak.Option{


[golangci] _{reported by reviewdog 🐶}
goleakIgnoreOptions is a global variable (gochecknoglobals)

hlts2

LGTM

vankichi

LGTM

kevindiu · 2020-06-15T07:11:06Z

@kpango This PR implements the test case of internal/safety test cases.
Since this PR customized the testing code by adding the defer() function to test the recover() result.
I think the similar test case may exists on other packages, I think it is better for you to review this changes.

I added the following code to the test code:
Line 27-36: https://github.com/vdaas/vald/pull/464/files#diff-d24e608c08457561fd4b754590a38847R27-R36
Line 44: https://github.com/vdaas/vald/pull/464/files#diff-d24e608c08457561fd4b754590a38847R44
Line 131-144: https://github.com/vdaas/vald/pull/464/files#diff-d24e608c08457561fd4b754590a38847R131-R144

It is not urgent but can you please review it in this week (6/19)?

kevindiu · 2020-06-15T07:22:33Z

/rebase

vdaas-ci · 2020-06-15T07:23:12Z

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

Co-authored-by: Hiroto Funakoshi <[email protected]>

Co-authored-by: Kiichiro YUKAWA <[email protected]>

kpango

LGTM

kpango · 2020-06-15T07:31:28Z

/format
/approve

vdaas-ci · 2020-06-15T07:33:14Z

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

vdaas-ci

[APPROVED] This PR is approved by kpango.

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

hlts2 previously approved these changes Jun 10, 2020

View reviewed changes

kevindiu marked this pull request as ready for review June 10, 2020 08:25

kevindiu requested a review from vankichi June 10, 2020 08:25

kevindiu changed the title ~~[Draft] Test/internal/safety~~ Test/internal/safety Jun 10, 2020

kevindiu changed the title ~~Test/internal/safety~~ Fix failed test case of internal/safety package Jun 10, 2020

vdaas-ci dismissed hlts2’s stale review via 7fd531b June 10, 2020 10:35

vdaas-ci force-pushed the test/internal/safety branch from 30b0bcd to 7fd531b Compare June 10, 2020 10:35

vankichi previously approved these changes Jun 11, 2020

View reviewed changes

vdaas-ci previously approved these changes Jun 11, 2020

View reviewed changes

vdaas-ci dismissed stale reviews from vankichi and themself via 99895a7 June 11, 2020 05:43

vdaas-ci force-pushed the test/internal/safety branch from 7fd531b to 99895a7 Compare June 11, 2020 05:43

hlts2 reviewed Jun 11, 2020

View reviewed changes

docs/contributing/coding-style.md Outdated Show resolved Hide resolved

vankichi reviewed Jun 15, 2020

View reviewed changes

kevindiu dismissed hlts2’s stale review via ba01a2b June 15, 2020 05:13

github-actions bot reviewed Jun 15, 2020

View reviewed changes

hlts2 previously approved these changes Jun 15, 2020

View reviewed changes

vankichi previously approved these changes Jun 15, 2020

View reviewed changes

kevindiu requested a review from kpango June 15, 2020 06:55

kevindiu added the status/author-review label Jun 15, 2020

kevindiu and others added 10 commits June 15, 2020 07:23

fix test case

bfa2efd

fix

fb2c596

genereate missing test code

07c3a51

add coding doc

084e304

fix doc

ea1c9c5

Apply suggestions from code review

220e01e

Co-authored-by: Hiroto Funakoshi <[email protected]>

Apply suggestions from code review

0f90ca5

Co-authored-by: Kiichiro YUKAWA <[email protected]>

Apply suggestions from code review

f987fee

Co-authored-by: Kiichiro YUKAWA <[email protected]>

fix formatting

562f84d

add title

60bb2fb

vdaas-ci dismissed stale reviews from vankichi and hlts2 via 60bb2fb June 15, 2020 07:23

vdaas-ci force-pushed the test/internal/safety branch from a9588d8 to 60bb2fb Compare June 15, 2020 07:23

kpango approved these changes Jun 15, 2020

View reviewed changes

vdaas-ci approved these changes Jun 15, 2020

View reviewed changes

kpango merged commit 32a86e3 into master Jun 15, 2020

kpango deleted the test/internal/safety branch June 15, 2020 07:36

vdaas-ci mentioned this pull request Jun 18, 2020

[patch] ✨ Add Stackdriver Monitoring, Tracing and Profiler support #479

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix failed test case of internal/safety package #464

Fix failed test case of internal/safety package #464

kevindiu commented Jun 10, 2020

pull-assistant bot commented Jun 10, 2020 •

edited

Loading

kevindiu commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

codecov bot commented Jun 10, 2020 •

edited

Loading

hlts2 left a comment •

edited

Loading

vankichi commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

vankichi left a comment

vankichi commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vdaas-ci left a comment

vankichi commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vankichi left a comment

github-actions bot Jun 15, 2020

hlts2 left a comment

vankichi left a comment

kevindiu commented Jun 15, 2020

kevindiu commented Jun 15, 2020

vdaas-ci commented Jun 15, 2020

kpango left a comment

kpango commented Jun 15, 2020

vdaas-ci commented Jun 15, 2020

vdaas-ci left a comment

Fix failed test case of internal/safety package #464

Fix failed test case of internal/safety package #464

Conversation

kevindiu commented Jun 10, 2020

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

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

kevindiu commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

codecov bot commented Jun 10, 2020 • edited Loading

Codecov Report

hlts2 left a comment • edited Loading

Choose a reason for hiding this comment

vankichi commented Jun 10, 2020

vdaas-ci commented Jun 10, 2020

vankichi left a comment

Choose a reason for hiding this comment

vankichi commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

vankichi commented Jun 11, 2020

vdaas-ci commented Jun 11, 2020

vankichi left a comment

Choose a reason for hiding this comment

github-actions bot Jun 15, 2020

Choose a reason for hiding this comment

hlts2 left a comment

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

kevindiu commented Jun 15, 2020

kevindiu commented Jun 15, 2020

vdaas-ci commented Jun 15, 2020

kpango left a comment

Choose a reason for hiding this comment

kpango commented Jun 15, 2020

vdaas-ci commented Jun 15, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

pull-assistant bot commented Jun 10, 2020 •

edited

Loading

codecov bot commented Jun 10, 2020 •

edited

Loading

hlts2 left a comment •

edited

Loading