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

refactor internal package (net, file, logger) #1768

Merged

Conversation

kpango
Copy link
Collaborator

@kpango kpango commented Aug 16, 2022

Signed-off-by: kpango kpango@vdaas.org

Description:

SSIA

Related Issue:

How Has This Been Tested?:

Environment:

  • Go Version: 1.18.3
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 1.14.6

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.

@kpango kpango changed the title refactor internal packages add os.Rename try phase for internal/file.CopyWithPerm Aug 16, 2022
@kpango kpango changed the title add os.Rename try phase for internal/file.CopyWithPerm [Pending] add os.Rename try phase for internal/file.CopyWithPerm Aug 16, 2022
@kpango kpango force-pushed the refactor/internal/add-rename-operation-trying-for-file-copy branch from 4f42380 to d8b02b5 Compare September 7, 2022 03:38
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 7, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: cdec188
Status: ✅  Deploy successful!
Preview URL: https://5c76bd5f.vald.pages.dev
Branch Preview URL: https://refactor-internal-add-rename.vald.pages.dev

View logs

@codecov
Copy link

codecov bot commented Sep 7, 2022

Codecov Report

Base: 30.27% // Head: 30.32% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (e8d8db8) compared to base (8ec79af).
Patch coverage: 61.97% of modified lines in pull request are covered.

❗ Current head e8d8db8 differs from pull request most recent head cdec188. Consider uploading reports for the commit cdec188 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1768      +/-   ##
==========================================
+ Coverage   30.27%   30.32%   +0.04%     
==========================================
  Files         373      373              
  Lines       33713    33838     +125     
==========================================
+ Hits        10207    10261      +54     
- Misses      23113    23171      +58     
- Partials      393      406      +13     
Impacted Files Coverage Δ
internal/errors/tls.go 0.00% <0.00%> (ø)
internal/file/file.go 11.93% <0.00%> (-0.10%) ⬇️
internal/net/grpc/pool/pool.go 0.00% <0.00%> (ø)
internal/strings/strings.go 81.81% <ø> (ø)
internal/tls/tls.go 79.03% <ø> (ø)
internal/net/net.go 85.71% <50.00%> (-3.39%) ⬇️
internal/net/dialer.go 76.94% <71.32%> (-11.78%) ⬇️
internal/info/info.go 88.82% <100.00%> (+0.49%) ⬆️
internal/net/option.go 88.50% <100.00%> (+0.27%) ⬆️
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

for _, path := range strings.Split(file[idx+goModLen:], "/") {
left, right, ok := strings.Cut(path, "@")
if ok {
if strings.Count(right, "-") > 2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 2, in detected (gomnd)

url = strings.Replace(strings.SplitN(file, "go/src/", 2)[1]+"#L"+strconv.Itoa(line), valdRepo, "https://"+valdRepo+"/blob/"+i.detail.GitCommit, -1)
case func() bool {
idx = strings.Index(file, goSrc)
return idx >= 0 && strings.Index(file, valdRepo) >= 0
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
S1003: should use strings.Contains(file, valdRepo) instead (gosimple)

},
func() test {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(200)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
"200" can be replaced by http.StatusOK (usestdlibvars)

}(),
func() test {
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(200)
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
"200" can be replaced by http.StatusOK (usestdlibvars)

kpango added a commit that referenced this pull request Sep 8, 2022
* fix test error for PR#1768

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

* fix index test build error

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* regenerate backup files to fix info test error

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* disable log for save index test to avoid error log

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* fix info test by correct branch name & fix Get() impl

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* Fix dialer test error (#1784)

* fix dialer test error

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* revert unnecessary changes

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* correct dialer test impl

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* Fix formatter

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* fix comment

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* remove backup file for test

Signed-off-by: kevindiu <kevindiujp@gmail.com>

Signed-off-by: kpango <kpango@vdaas.org>
Signed-off-by: kevindiu <kevindiujp@gmail.com>
Co-authored-by: kpango <kpango@vdaas.org>
@kpango kpango changed the title [Pending] add os.Rename try phase for internal/file.CopyWithPerm add os.Rename try phase for internal/file.CopyWithPerm Sep 8, 2022

<a name="vald-v1-Filter"></a>

### Filter
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Suggestions: Filter
Rule: https://community.languagetool.org/rule/show/ENGLISH_WORD_REPEAT_RULE?lang=en-US
Category: MISC


<a name="vald-v1-Insert"></a>

### Insert
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Suggestions: Insert
Rule: https://community.languagetool.org/rule/show/ENGLISH_WORD_REPEAT_RULE?lang=en-US
Category: MISC

@@ -99,7 +99,7 @@ Configuration
| agent.ngt.vqueue.insert_buffer_pool_size | int | `10000` | insert slice pool buffer size |
| agent.nodeName | string | `""` | node name |
| agent.nodeSelector | object | `{}` | node selector |
| agent.observability | object | `{"jaeger":{"service_name":"vald-agent-ngt"},"stackdriver":{"profiler":{"service":"vald-agent-ngt"}}}` | observability config (overrides defaults.observability) |
| agent.observability | object | `{"jaeger":{"service_name":"vald-agent-ngt"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -99,7 +99,7 @@
| agent.ngt.vqueue.insert_buffer_pool_size | int | `10000` | insert slice pool buffer size |
| agent.nodeName | string | `""` | node name |
| agent.nodeSelector | object | `{}` | node selector |
| agent.observability | object | `{"jaeger":{"service_name":"vald-agent-ngt"},"stackdriver":{"profiler":{"service":"vald-agent-ngt"}}}` | observability config (overrides defaults.observability) |
| agent.observability | object | `{"jaeger":{"service_name":"vald-agent-ngt"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -212,7 +212,7 @@
| agent.sidecar.initContainerEnabled | bool | `false` | sidecar on initContainer mode enabled. |
| agent.sidecar.logging | object | `{}` | logging config (overrides defaults.logging) |
| agent.sidecar.name | string | `"vald-agent-sidecar"` | name of agent sidecar |
| agent.sidecar.observability | object | `{"jaeger":{"service_name":"vald-agent-sidecar"},"stackdriver":{"profiler":{"service":"vald-agent-sidecar"}}}` | observability config (overrides defaults.observability) |
| agent.sidecar.observability | object | `{"jaeger":{"service_name":"vald-agent-sidecar"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -597,7 +566,7 @@
| discoverer.name | string | `"vald-discoverer"` | name of discoverer deployment |
| discoverer.nodeName | string | `""` | node name |
| discoverer.nodeSelector | object | `{}` | node selector |
| discoverer.observability | object | `{"jaeger":{"service_name":"vald-discoverer"},"stackdriver":{"profiler":{"service":"vald-discoverer"}}}` | observability config (overrides defaults.observability) |
| discoverer.observability | object | `{"jaeger":{"service_name":"vald-discoverer"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -597,7 +566,7 @@
| discoverer.name | string | `"vald-discoverer"` | name of discoverer deployment |
| discoverer.nodeName | string | `""` | node name |
| discoverer.nodeSelector | object | `{}` | node selector |
| discoverer.observability | object | `{"jaeger":{"service_name":"vald-discoverer"},"stackdriver":{"profiler":{"service":"vald-discoverer"}}}` | observability config (overrides defaults.observability) |
| discoverer.observability | object | `{"jaeger":{"service_name":"vald-discoverer"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -663,7 +632,7 @@
| gateway.filter.name | string | `"vald-filter-gateway"` | name of filter gateway deployment |
| gateway.filter.nodeName | string | `""` | node name |
| gateway.filter.nodeSelector | object | `{}` | node selector |
| gateway.filter.observability | object | `{"jaeger":{"service_name":"vald-filter-gateway"},"stackdriver":{"profiler":{"service":"vald-filter-gateway"}}}` | observability config (overrides defaults.observability) |
| gateway.filter.observability | object | `{"jaeger":{"service_name":"vald-filter-gateway"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -721,7 +690,7 @@
| gateway.lb.name | string | `"vald-lb-gateway"` | name of gateway deployment |
| gateway.lb.nodeName | string | `""` | node name |
| gateway.lb.nodeSelector | object | `{}` | node selector |
| gateway.lb.observability | object | `{"jaeger":{"service_name":"vald-lb-gateway"},"stackdriver":{"profiler":{"service":"vald-lb-gateway"}}}` | observability config (overrides defaults.observability) |
| gateway.lb.observability | object | `{"jaeger":{"service_name":"vald-lb-gateway"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@@ -775,7 +744,7 @@
| manager.index.name | string | `"vald-manager-index"` | name of index manager deployment |
| manager.index.nodeName | string | `""` | node name |
| manager.index.nodeSelector | object | `{}` | node selector |
| manager.index.observability | object | `{"jaeger":{"service_name":"vald-manager-index"},"stackdriver":{"profiler":{"service":"vald-manager-index"}}}` | observability config (overrides defaults.observability) |
| manager.index.observability | object | `{"jaeger":{"service_name":"vald-manager-index"}}` | observability config (overrides defaults.observability) |
Copy link
Contributor

Choose a reason for hiding this comment

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

[LanguageTool] reported by reviewdog 🐶
If a new sentence starts here, add a space and start with an uppercase letter. (LC_AFTER_PERIOD[1])
Suggestions: Observability, observability
Rule: https://community.languagetool.org/rule/show/LC_AFTER_PERIOD?lang=en-US&subId=1
Category: CASING

@github-actions github-actions bot removed the size/XXL label Sep 8, 2022
kpango added a commit that referenced this pull request Sep 8, 2022
* fix test error for PR#1768

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

* fix index test build error

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* regenerate backup files to fix info test error

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* disable log for save index test to avoid error log

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* fix info test by correct branch name & fix Get() impl

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* Fix dialer test error (#1784)

* fix dialer test error

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* revert unnecessary changes

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* correct dialer test impl

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* Fix formatter

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* fix comment

Signed-off-by: kevindiu <kevindiujp@gmail.com>

* remove backup file for test

Signed-off-by: kevindiu <kevindiujp@gmail.com>

Signed-off-by: kpango <kpango@vdaas.org>
Signed-off-by: kevindiu <kevindiujp@gmail.com>
Co-authored-by: kpango <kpango@vdaas.org>
Signed-off-by: kpango <kpango@vdaas.org>
@kpango kpango force-pushed the refactor/internal/add-rename-operation-trying-for-file-copy branch from bd5fb7e to 9980640 Compare September 8, 2022 03:09
kpango added a commit that referenced this pull request Sep 8, 2022
Fix test from #1768 (#1734)

Signed-off-by: kpango <kpango@vdaas.org>
@kpango kpango force-pushed the refactor/internal/add-rename-operation-trying-for-file-copy branch from 9980640 to e8d8db8 Compare September 8, 2022 03:10
@kpango kpango marked this pull request as ready for review September 8, 2022 03:10
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

golangci

internal/observability/client/google/option_test.go|30 col 3| field apiKey is unused (unused)
internal/observability/client/google/option_test.go|375 col 3| field endpoint is unused (unused)
internal/observability/client/google/option_test.go|260 col 3| field path is unused (unused)
internal/observability/client/google/option_test.go|835 col 3| field ua is unused (unused)
internal/observability/client/google/option_test.go|145 col 3| field audiences is unused (unused)
internal/observability/client/google/option_test.go|490 col 3| field qp is unused (unused)
internal/observability/client/google/option_test.go|950 col 3| field json is unused (unused)
internal/info/info.go|265 col 9| copylocks: get passes lock by value: github.com/vdaas/vald/internal/info.info contains sync.Once contains sync.Mutex (govet)
internal/net/dialer_test.go|961 col 16| test helper function should start from t.Helper() (thelper)
internal/net/dialer_test.go|997 col 17| test helper function should start from t.Helper() (thelper)
internal/net/dialer_test.go|1757 col 16| parameter *testing.T should have name t (thelper)

*/

tests := []test{
// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/observability/client/google/option_test.go:72: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

},
*/

// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/observability/client/google/option_test.go:85: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

*/

tests := []test{
// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/observability/client/google/option_test.go:187: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

},
*/

// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/observability/client/google/option_test.go:200: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

*/

tests := []test{
// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/observability/client/google/option_test.go:302: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

type args struct {
enabled bool
}
type want struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
type want is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
rr string
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field rr is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
scopes []string
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field scopes is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
enabled bool
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field enabled is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
enabled bool
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field enabled is unused (unused)

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Sep 8, 2022

[WARNING:INTCFG] Changes in interal/config may require you to change Helm charts. Please check.

hlts2
hlts2 previously approved these changes Sep 8, 2022
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

vankichi
vankichi previously approved these changes Sep 8, 2022
datelier
datelier previously approved these changes Sep 8, 2022
Signed-off-by: kpango <kpango@vdaas.org>
@kpango kpango dismissed stale reviews from datelier, vankichi, and hlts2 via cdec188 September 8, 2022 06:02
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM


func TestVerifyTestMain(t *testing.T) {
type args struct {
m TestingM
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
undeclared name: TestingM (typecheck)

type test struct {
name string
args args
want want
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field want is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
json string
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field json is unused (unused)

type test struct {
name string
args args
want want
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field want is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
audiences []string
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field audiences is unused (unused)

// Change interface type to the type of object you are testing
type T = interface{}
type args struct {
endpoint string
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field endpoint is unused (unused)


func (i info) get() Detail {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
copylocks: get passes lock by value: github.com/vdaas/vald/internal/info.info contains sync.Once contains sync.Mutex (govet)

return errors.New("cache value is deleted")
}
return nil
},
afterFunc: func(args) {
afterFunc: func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
test helper function should start from t.Helper() (thelper)

WithDNSCache(c),
WithTLS(tls),
},
beforeFunc: func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
test helper function should start from t.Helper() (thelper)

}
return nil
},
afterFunc: func(a args) {
afterFunc: func(t1 *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
parameter *testing.T should have name t (thelper)

@kpango kpango merged commit 1b6a344 into main Sep 8, 2022
@kpango kpango deleted the refactor/internal/add-rename-operation-trying-for-file-copy branch September 8, 2022 06:12
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Sep 8, 2022

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
other images

return nil
}
tests := []test{
// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/test/goleak/goleak_test.go:109: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

},
*/

// TODO test cases
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
internal/test/goleak/goleak_test.go:122: Line contains TODO/BUG/FIXME: "TODO test cases" (godox)

type args struct {
rr string
}
type want struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
type want is unused (unused)

type test struct {
name string
args args
want want
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field want is unused (unused)

type test struct {
name string
args args
want want
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
field want is unused (unused)

@kevindiu kevindiu mentioned this pull request Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants