Add cassandra option test #644

kpango · 2020-08-27T03:35:21Z

Description:

This PR includes the cassandra_option test code, and some refactoring for writing the test code.
It is only the part of the option test.
It also contains the changes to update the coding guideline for FOP implementation and handling.

Reference: #638

Related Issue:

How Has This Been Tested?:

Environment:

Go Version: 1.14.4
Docker Version: 19.03.8
Kubernetes Version: 1.18.2
NGT Version: 1.12.0

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.

vdaas-ci · 2020-08-27T03:35:34Z

[CHATOPS:HELP] ChatOps commands.

🙆‍♀️ /approve - approve
💌 /changelog - add changelog comment
🍱 /format - format codes and add licenses
✅ /gen-test - generate test codes
🏷️ /label - add labels
⏪ /rebase - rebase master

pull-assistant · 2020-08-27T03:35:57Z

Best reviewed: commit by commit

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

add cassandra option test

fix

ttest

fix

...l/db/nosql/cassandra/option.go 67% changes removed in fix failed test and ...

fix int option

...l/db/nosql/cassandra/option.go 56% changes removed in fix failed test and ...

fix deepsource

🤖 Update license headers / Format go codes and yaml files

fix

🤖 Update license headers / Format go codes and yaml files

fix

Merge branch 'master' into test/internal/db/cassandra/option

...ithub/PULL_REQUEST_TEMPLATE.md 50% changes removed in Merge branch 'master...

...s/manager/index/configmap.yaml 67% changes removed in Merge branch 'master...

...PLATE/security_issue_report.md 50% changes removed in Merge branch 'master...

go.mod 44% changes removed in Merge branch 'master...

...b/ISSUE_TEMPLATE/bug_report.md 50% changes removed in Merge branch 'master...

update coding guidel... ... fix

Squashed 2 commits:

update coding guideline

fix

...l/db/nosql/cassandra/option.go 80% changes removed in fix

internal/errors/option.go 43% changes removed in fix

...s/contributing/coding-style.md 54% changes removed in update coding guidel...

fix

internal/errors/option.go 50% changes removed in fix

Merge branch 'master' into test/internal/db/cassandra/option

...8s/discoverer/clusterrole.yaml 67% changes removed in Merge branch 'master...

...overer/clusterrolebinding.yaml 67% changes removed in Merge branch 'master...

fix guideline

apply suggestion

internal/errors/option.go 85% changes removed in fix

fix

fix indent

update guideline

fix

internal/errors/option.go 60% changes removed in fix

fix

fix failed test and refactor

fix comment

use beforeFunc in test case

fix

update coding guideline

Merge branch 'master' into test/internal/db/cassandra/option

Powered by Pull Assistant. Last update 5af01e9 ... 8491247. Read the comment docs.

kevindiu · 2020-08-27T06:38:28Z

/rebase

vdaas-ci · 2020-08-27T06:39:08Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/db/cassandra/option

internal/db/nosql/cassandra/option_test.go

internal/db/nosql/cassandra/cassandra.go

codecov · 2020-08-27T06:48:04Z

Codecov Report

Merging #644 into master will increase coverage by 0.08%.
The diff coverage is 36.23%.

@@            Coverage Diff             @@
##           master     #644      +/-   ##
==========================================
+ Coverage   15.35%   15.43%   +0.08%     
==========================================
  Files         412      417       +5     
  Lines       21629    22177     +548     
==========================================
+ Hits         3321     3424     +103     
- Misses      18060    18518     +458     
+ Partials      248      235      -13

Impacted Files	Coverage Δ
internal/db/nosql/cassandra/cassandra.go	`0.00% <0.00%> (ø)`
internal/errors/option.go	`0.00% <0.00%> (ø)`
internal/db/nosql/cassandra/option.go	`30.07% <100.00%> (+20.26%)`	⬆️
internal/db/storage/blob/s3/writer/option.go	`100.00% <100.00%> (ø)`
internal/config/ngt.go	`100.00% <0.00%> (ø)`
pkg/agent/core/ngt/service/ngt.go	`0.00% <0.00%> (ø)`
pkg/agent/core/ngt/usecase/agentd.go	`0.00% <0.00%> (ø)`
pkg/tools/cli/loadtest/usecase/load.go	`0.00% <0.00%> (ø)`
pkg/tools/cli/loadtest/service/loader.go	`0.00% <0.00%> (ø)`
...ternal/observability/metrics/agent/core/ngt/ngt.go	`0.00% <0.00%> (ø)`
... and 9 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 8445611...8491247. Read the comment docs.

kevindiu · 2020-08-27T07:27:56Z

/rebase

vdaas-ci · 2020-08-27T07:28:30Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/db/cassandra/option

internal/db/nosql/cassandra/option_test.go

kevindiu · 2020-08-31T00:30:07Z

/rebase

vdaas-ci · 2020-08-31T00:30:43Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/db/cassandra/option

internal/db/nosql/cassandra/option.go

internal/db/nosql/cassandra/cassandra.go

internal/db/nosql/cassandra/option_test.go

hlts2

LGTM

vankichi

LGTM
but, it contains logic changes, please ask for authors.

internal/db/nosql/cassandra/option.go

internal/db/nosql/cassandra/option_test.go

hlts2

LGTM

internal/db/nosql/cassandra/option.go

internal/errors/option.go

internal/db/nosql/cassandra/option_test.go

vankichi · 2020-09-16T08:53:44Z

internal/db/nosql/cassandra/option_test.go

+			},
+		},
+		{
+			name: "set timeout success if the time format is invalid",


I think this pattern gets the error...

it is because I didn't fix this implementation.
https://github.com/vdaas/vald/blob/test/internal/db/cassandra/option/internal/db/nosql/cassandra/option.go#L124

could you please tell me the reason of this implementation?
maybe, I heard at once, but I forgot 😓

@vankichi There's no reason of this implementation. I haven't fix this implementation😂
Please let me fix it in next PR as changing it cause logic changes which requires approval from authors, which may cause time.

what should we do?

@vankichi can we ignore it in this PR and work on it in next PR?

internal/db/nosql/cassandra/option_test.go

vankichi · 2020-09-17T01:30:18Z

internal/db/nosql/cassandra/option_test.go

+			},
+		},
+		{
+			name: "set timeout success if the time format is invalid",


what should we do?

kpango · 2020-09-17T04:35:59Z

/rebase
/format

vdaas-ci · 2020-09-17T04:36:35Z

[REBASE] Rebase triggered by kpango for branch: test/internal/db/cassandra/option

vdaas-ci · 2020-09-17T04:36:40Z

[REBASE] Failed to rebase.

github-actions · 2020-09-17T04:50:06Z

docs/contributing/coding-style.md

-            return nil, errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+            werr := errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+
+            e := new(errors.ErrCriticalOption)


[LanguageTool] _{reported by reviewdog 🐶}
This sentence does not start with an uppercase letter (UPPERCASE_SENTENCE_START)
Suggestions: E
Rule: https://community.languagetool.org/rule/show/UPPERCASE_SENTENCE_START?lang=en-US
Category: CASING

github-actions · 2020-09-17T04:50:06Z

docs/contributing/coding-style.md

-            return nil, errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+            werr := errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+
+            e := new(errors.ErrCriticalOption)


[LanguageTool] _{reported by reviewdog 🐶}
Add a space between sentences (SENTENCE_WHITESPACE)
Suggestions: ErrCriticalOption
Rule: https://community.languagetool.org/rule/show/SENTENCE_WHITESPACE?lang=en-US
Category: TYPOGRAPHY

github-actions · 2020-09-17T04:50:06Z

docs/contributing/coding-style.md

+            werr := errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+
+            e := new(errors.ErrCriticalOption)
+			if errors.As(err, &e) {


[LanguageTool] _{reported by reviewdog 🐶}
Add a space between sentences (SENTENCE_WHITESPACE)
Suggestions: As
Rule: https://community.languagetool.org/rule/show/SENTENCE_WHITESPACE?lang=en-US
Category: TYPOGRAPHY

github-actions · 2020-09-17T04:50:06Z

docs/contributing/coding-style.md

+            werr := errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+
+            e := new(errors.ErrCriticalOption)
+			if errors.As(err, &e) {


[LanguageTool] _{reported by reviewdog 🐶}
Don't put a space after the opening parenthesis (COMMA_PARENTHESIS_WHITESPACE)
Suggestions: {
Rule: https://community.languagetool.org/rule/show/COMMA_PARENTHESIS_WHITESPACE?lang=en-US
Category: TYPOGRAPHY

kpango · 2020-09-17T05:06:19Z

/rebase
/format
/approve

vdaas-ci · 2020-09-17T05:06:59Z

[REBASE] Rebase triggered by kpango for branch: test/internal/db/cassandra/option

vdaas-ci · 2020-09-17T05:07:06Z

[REBASE] Failed to rebase.

kpango · 2020-09-17T05:09:28Z

/format
/approve

vdaas-ci · 2020-09-17T05:10:58Z

[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/XXL type/test Adding tests labels Aug 27, 2020

github-actions bot added the priority/low label Aug 27, 2020

vdaas-ci force-pushed the test/internal/db/cassandra/option branch from c4dbb6c to ed0a05b Compare August 27, 2020 06:39

github-actions bot reviewed Aug 27, 2020

View reviewed changes

internal/db/nosql/cassandra/option_test.go Outdated Show resolved Hide resolved

internal/db/nosql/cassandra/cassandra.go Show resolved Hide resolved

vankichi added the status/need-fix label Aug 27, 2020

vankichi reviewed Aug 27, 2020

View reviewed changes

internal/db/nosql/cassandra/option_test.go Outdated Show resolved Hide resolved

kevindiu removed the status/need-fix label Aug 31, 2020

kevindiu requested review from hlts2 and vankichi August 31, 2020 00:48

hlts2 reviewed Aug 31, 2020

View reviewed changes

internal/db/nosql/cassandra/option.go Show resolved Hide resolved

hlts2 reviewed Aug 31, 2020

View reviewed changes

internal/db/nosql/cassandra/option.go Show resolved Hide resolved

kevindiu requested a review from hlts2 August 31, 2020 02:51

github-actions bot reviewed Aug 31, 2020

View reviewed changes

hlts2 previously approved these changes Aug 31, 2020

View reviewed changes

vankichi previously approved these changes Sep 1, 2020

View reviewed changes

internal/db/nosql/cassandra/option.go Outdated Show resolved Hide resolved

kevindiu added the status/author-review label Sep 1, 2020

kevindiu requested review from rinx and kmrmt September 1, 2020 03:23

fix failed test and refactor

133eb82

kevindiu dismissed hlts2’s stale review via 133eb82 September 16, 2020 07:19

github-actions bot reviewed Sep 16, 2020

View reviewed changes

internal/db/nosql/cassandra/option_test.go Show resolved Hide resolved

hlts2 previously approved these changes Sep 16, 2020

View reviewed changes

kevindiu changed the title ~~[WIP] Add cassandra option test~~ Add cassandra option test Sep 16, 2020

vankichi reviewed Sep 16, 2020

View reviewed changes

fix comment

daf1faf

kevindiu dismissed hlts2’s stale review via daf1faf September 16, 2020 09:09

use beforeFunc in test case

5cf14cd

github-actions bot reviewed Sep 16, 2020

View reviewed changes

internal/db/nosql/cassandra/option_test.go Outdated Show resolved Hide resolved

kevindiu requested a review from vankichi September 17, 2020 00:59

vankichi reviewed Sep 17, 2020

View reviewed changes

kevindiu added 2 commits September 17, 2020 10:54

fix

6b33558

fix

28d24ce

hlts2 previously approved these changes Sep 17, 2020

View reviewed changes

update coding guideline

83fe277

kevindiu dismissed hlts2’s stale review via 83fe277 September 17, 2020 04:47

github-actions bot reviewed Sep 17, 2020

View reviewed changes

Merge branch 'master' into test/internal/db/cassandra/option

8491247

vdaas-ci approved these changes Sep 17, 2020

View reviewed changes

kpango merged commit 1bd41ef into master Sep 17, 2020

kpango deleted the test/internal/db/cassandra/option branch September 17, 2020 05:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cassandra option test #644

Add cassandra option test #644

kpango commented Aug 27, 2020 •

edited by kevindiu

Loading

vdaas-ci commented Aug 27, 2020

pull-assistant bot commented Aug 27, 2020 •

edited

Loading

kevindiu commented Aug 27, 2020

vdaas-ci commented Aug 27, 2020

codecov bot commented Aug 27, 2020 •

edited

Loading

kevindiu commented Aug 27, 2020

vdaas-ci commented Aug 27, 2020

kevindiu commented Aug 31, 2020

vdaas-ci commented Aug 31, 2020

hlts2 left a comment

vankichi left a comment

hlts2 left a comment

vankichi Sep 16, 2020

vankichi Sep 16, 2020

kevindiu Sep 16, 2020

vankichi Sep 16, 2020

kevindiu Sep 16, 2020

vankichi Sep 17, 2020

kevindiu Sep 17, 2020

vankichi Sep 17, 2020

kpango commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

github-actions bot Sep 17, 2020

github-actions bot Sep 17, 2020

github-actions bot Sep 17, 2020

github-actions bot Sep 17, 2020

kpango commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

kpango commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

vdaas-ci left a comment

Add cassandra option test #644

Add cassandra option test #644

Conversation

kpango commented Aug 27, 2020 • edited by kevindiu Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

vdaas-ci commented Aug 27, 2020

pull-assistant bot commented Aug 27, 2020 • edited Loading

kevindiu commented Aug 27, 2020

vdaas-ci commented Aug 27, 2020

codecov bot commented Aug 27, 2020 • edited Loading

Codecov Report

kevindiu commented Aug 27, 2020

vdaas-ci commented Aug 27, 2020

kevindiu commented Aug 31, 2020

vdaas-ci commented Aug 31, 2020

hlts2 left a comment

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

hlts2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpango commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

github-actions bot Sep 17, 2020

Choose a reason for hiding this comment

github-actions bot Sep 17, 2020

Choose a reason for hiding this comment

github-actions bot Sep 17, 2020

Choose a reason for hiding this comment

github-actions bot Sep 17, 2020

Choose a reason for hiding this comment

kpango commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

kpango commented Sep 17, 2020

vdaas-ci commented Sep 17, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

kpango commented Aug 27, 2020 •

edited by kevindiu

Loading

pull-assistant bot commented Aug 27, 2020 •

edited

Loading

codecov bot commented Aug 27, 2020 •

edited

Loading