Add test for `internal/file/watch` #526

hlts2 · 2020-06-29T08:18:15Z

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

Description:

I added test case for internal/file/watch

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-29T08:18:17Z

Best reviewed: commit by commit

Optimal code review plan (2 warnings)

feat: add test case

fix: refactor

feat: add test case

fix: refactor test case

fix: apply suggestion

...ernal/file/watch/watch_test.go 50% changes removed in fix: apply suggestio...

fix: apply suggestion

fix: apply suggestion and add test case

fix: logic for mac

fix: apply suggestion

internal/file/watch/watch.go 75% changes removed in fix: apply suggestio...

fix: apply suggestion

Powered by Pull Assistant. Last update bac7350 ... df0bbbc. Read the comment docs.

internal/file/watch/watch_test.go

codecov · 2020-06-29T08:26:28Z

Codecov Report

Merging #526 into master will increase coverage by 2.93%.
The diff coverage is 8.69%.

@@            Coverage Diff             @@
##           master     #526      +/-   ##
==========================================
+ Coverage    7.84%   10.77%   +2.93%     
==========================================
  Files         388      403      +15     
  Lines       19945    20939     +994     
==========================================
+ Hits         1565     2257     +692     
- Misses      18156    18414     +258     
- Partials      224      268      +44

Impacted Files	Coverage Δ
internal/cache/cacher/cacher.go	`100.00% <ø> (+100.00%)`	⬆️
internal/cache/gache/gache.go	`0.00% <0.00%> (ø)`
internal/cache/gache/option.go	`0.00% <0.00%> (-6.67%)`	⬇️
internal/client/discoverer/discover.go	`0.00% <0.00%> (ø)`
internal/config/blob.go	`0.00% <0.00%> (ø)`
internal/config/grpc.go	`0.00% <0.00%> (ø)`
internal/config/log.go	`100.00% <ø> (+100.00%)`	⬆️
internal/config/mysql.go	`100.00% <ø> (+100.00%)`	⬆️
internal/config/ngt.go	`100.00% <ø> (+100.00%)`	⬆️
internal/config/observability.go	`0.00% <0.00%> (ø)`
... and 150 more

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 0c0505e...df0bbbc. Read the comment docs.

hlts2 · 2020-06-30T02:48:48Z

/rebase

vdaas-ci · 2020-06-30T02:49:21Z

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

hlts2 · 2020-06-30T06:38:29Z

/rebase

vdaas-ci · 2020-06-30T06:39:01Z

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

internal/file/watch/watch_test.go

hlts2 · 2020-07-06T03:28:19Z

/rebase

vdaas-ci · 2020-07-06T03:28:53Z

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

kevindiu · 2020-07-06T04:47:59Z

do we need author review for this PR?

hlts2 · 2020-07-06T06:17:09Z

@kevindiu

do we need author review for this PR?

I don't think it is necessary.
Because I wrote code with @kpango (pair programming).

kevindiu · 2020-07-06T06:53:16Z

@kevindiu

do we need author review for this PR?

I don't think it is necessary.
Because I wrote code with @kpango (pair programming).

OK!

internal/file/watch/option_test.go

vankichi · 2020-07-10T03:11:47Z

internal/file/watch/watch.go

+	dirs := make([]string, 0, len(w.dirs))
+	for d := range w.dirs {
+		dirs = append(dirs, d)
+	}


let me tell about why, please?

sure.
map type is reference type in golang.
so Even if we take the lock w.dirs, it changes when the referrer changes.
When we copy the value of w.dirs, the instantaneous state is copied, so even if the referrer changes, the copy is not affected.

vankichi · 2020-07-10T03:12:43Z

internal/file/watch/watch.go

+				iw, err := w.init()
+				if err == nil {
+					w = iw
+				}


same. Please tell me the reason.

sure.
if w.init returns error, w become nil.
and when roop continues, nil pointer exception will occur.

vankichi

LGTM

vankichi · 2020-07-10T05:04:52Z

/rebase
/format
/approve

vdaas-ci · 2020-07-10T05:05:36Z

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

vdaas-ci · 2020-07-10T05:09:09Z

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

vdaas-ci

[APPROVED] This PR is approved by vankichi.

vankichi · 2020-07-10T05:13:16Z

@hlts2
When you update some logics, please mention about that and author review is required or not at the PR description field.

vankichi · 2020-07-13T00:41:03Z

/rebase
/format
/approve

vdaas-ci · 2020-07-13T00:41:46Z

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

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

vdaas-ci · 2020-07-13T00:45:02Z

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

vdaas-ci

[APPROVED] This PR is approved by vankichi.

hlts2 self-assigned this Jun 29, 2020

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

github-actions bot reviewed Jun 29, 2020

View reviewed changes

internal/file/watch/watch_test.go Outdated Show resolved Hide resolved

internal/file/watch/watch_test.go Show resolved Hide resolved

internal/file/watch/watch_test.go Outdated Show resolved Hide resolved

hlts2 changed the title ~~[WIP] add test for internal/file/watch~~ [WIP] [draft] add test for internal/file/watch Jun 30, 2020

github-actions bot added size/XL and removed size/L labels Jun 30, 2020

vdaas-ci force-pushed the test/internal/file_watch branch from 0ee7c66 to dfe8b39 Compare June 30, 2020 06:39

github-actions bot reviewed Jun 30, 2020

View reviewed changes

internal/file/watch/watch_test.go Show resolved Hide resolved

internal/file/watch/watch_test.go Show resolved Hide resolved

github-actions bot reviewed Jul 1, 2020

View reviewed changes

internal/file/watch/watch_test.go Show resolved Hide resolved

internal/file/watch/watch_test.go Show resolved Hide resolved

github-actions bot added size/XXL and removed size/XL labels Jul 1, 2020

vdaas-ci force-pushed the test/internal/file_watch branch from e462d65 to d86811c Compare July 6, 2020 03:29

hlts2 changed the title ~~[WIP] [draft] add test for internal/file/watch~~ [draft] add test for internal/file/watch Jul 6, 2020

hlts2 marked this pull request as ready for review July 6, 2020 06:27

hlts2 changed the title ~~[draft] add test for internal/file/watch~~ Add test for internal/file/watch Jul 6, 2020

hlts2 requested a review from vankichi July 9, 2020 08:59

vankichi reviewed Jul 10, 2020

View reviewed changes

internal/file/watch/option_test.go Show resolved Hide resolved

vankichi reviewed Jul 10, 2020

View reviewed changes

vdaas-ci dismissed kevindiu’s stale review via 2ce46a0 July 10, 2020 05:05

vdaas-ci force-pushed the test/internal/file_watch branch from 2e950df to 2ce46a0 Compare July 10, 2020 05:05

vdaas-ci previously approved these changes Jul 10, 2020

View reviewed changes

hlts2 added 11 commits July 13, 2020 00:41

feat: add test case

bac7350

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

feat: add test case

b68e434

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

fix: refactor

cc460cb

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

feat: add test case

2dffc8d

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

fix: refactor test case

8d766a7

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

fix: apply suggestion

36c8c6d

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

fix: apply suggestion

f4de7ce

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

fix: apply suggestion and add test case

111c96b

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

fix: logic for mac

5488ea4

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

fix: apply suggestion

4e68a62

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

fix: apply suggestion

df0bbbc

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

vdaas-ci dismissed their stale review via df0bbbc July 13, 2020 00:41

vdaas-ci force-pushed the test/internal/file_watch branch from 2ce46a0 to df0bbbc Compare July 13, 2020 00:41

vdaas-ci approved these changes Jul 13, 2020

View reviewed changes

vankichi merged commit 634e3e5 into master Jul 13, 2020

vankichi deleted the test/internal/file_watch branch July 13, 2020 00:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test for `internal/file/watch` #526

Add test for `internal/file/watch` #526

hlts2 commented Jun 29, 2020 •

edited

Loading

pull-assistant bot commented Jun 29, 2020 •

edited

Loading

codecov bot commented Jun 29, 2020 •

edited

Loading

hlts2 commented Jun 30, 2020

vdaas-ci commented Jun 30, 2020

hlts2 commented Jun 30, 2020

vdaas-ci commented Jun 30, 2020

hlts2 commented Jul 6, 2020

vdaas-ci commented Jul 6, 2020

kevindiu commented Jul 6, 2020

hlts2 commented Jul 6, 2020

kevindiu commented Jul 6, 2020

vankichi Jul 10, 2020

hlts2 Jul 10, 2020

vankichi Jul 10, 2020

hlts2 Jul 10, 2020

vankichi left a comment

vankichi commented Jul 10, 2020

vdaas-ci commented Jul 10, 2020

vdaas-ci commented Jul 10, 2020

vdaas-ci left a comment

vankichi commented Jul 10, 2020

vankichi commented Jul 13, 2020

vdaas-ci commented Jul 13, 2020

vdaas-ci commented Jul 13, 2020

vdaas-ci left a comment

Add test for internal/file/watch #526

Add test for internal/file/watch #526

Conversation

hlts2 commented Jun 29, 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 29, 2020 • edited Loading

codecov bot commented Jun 29, 2020 • edited Loading

Codecov Report

hlts2 commented Jun 30, 2020

vdaas-ci commented Jun 30, 2020

hlts2 commented Jun 30, 2020

vdaas-ci commented Jun 30, 2020

hlts2 commented Jul 6, 2020

vdaas-ci commented Jul 6, 2020

kevindiu commented Jul 6, 2020

hlts2 commented Jul 6, 2020

kevindiu commented Jul 6, 2020

vankichi Jul 10, 2020

Choose a reason for hiding this comment

hlts2 Jul 10, 2020

Choose a reason for hiding this comment

vankichi Jul 10, 2020

Choose a reason for hiding this comment

hlts2 Jul 10, 2020

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

vankichi commented Jul 10, 2020

vdaas-ci commented Jul 10, 2020

vdaas-ci commented Jul 10, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

vankichi commented Jul 10, 2020

vankichi commented Jul 13, 2020

vdaas-ci commented Jul 13, 2020

vdaas-ci commented Jul 13, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

Add test for `internal/file/watch` #526

Add test for `internal/file/watch` #526

hlts2 commented Jun 29, 2020 •

edited

Loading

pull-assistant bot commented Jun 29, 2020 •

edited

Loading

codecov bot commented Jun 29, 2020 •

edited

Loading