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

fix failed test #561

Merged
merged 3 commits into from
Jul 9, 2020
Merged

fix failed test #561

merged 3 commits into from
Jul 9, 2020

Conversation

vankichi
Copy link
Contributor

@vankichi vankichi commented Jul 8, 2020

Signed-off-by: vankichi [email protected]

Description:

I fixed below tests which are failed.

  • internal/log/glg/glg_test
  • internal/servers/server/option_tset

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.

@vankichi vankichi requested review from kevindiu and hlts2 July 8, 2020 08:02
@pull-assistant
Copy link

pull-assistant bot commented Jul 8, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     ✅ 🐛 fix failed test

     ⬆️ update

     Revert ":arrow_up: update"

Powered by Pull Assistant. Last update c033293 ... a96132e. Read the comment docs.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 8, 2020

[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

@codecov
Copy link

codecov bot commented Jul 8, 2020

Codecov Report

Merging #561 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #561      +/-   ##
==========================================
- Coverage   10.17%   10.17%   -0.01%     
==========================================
  Files         403      403              
  Lines       20874    20874              
==========================================
- Hits         2124     2123       -1     
- Misses      18490    18491       +1     
  Partials      260      260              
Impacted Files Coverage Δ
internal/errgroup/group.go 93.42% <0.00%> (-1.32%) ⬇️

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 a697749...a96132e. Read the comment docs.

@kevindiu
Copy link
Contributor

kevindiu commented Jul 8, 2020

seems the CI still failed.

@vankichi
Copy link
Contributor Author

vankichi commented Jul 8, 2020

It is because of panic pattern test 😓
I think we have to make decision about it.

@vankichi
Copy link
Contributor Author

vankichi commented Jul 9, 2020

/rebase

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 9, 2020

[REBASE] Rebase triggered by vankichi for branch: test/internal/fix-failed-internal-test

@vdaas-ci vdaas-ci force-pushed the test/internal/fix-failed-internal-test branch from 5248e07 to e774567 Compare July 9, 2020 02:41
@kevindiu
Copy link
Contributor

kevindiu commented Jul 9, 2020

this error occurs...

# github.com/vdaas/vald/internal/io [github.com/vdaas/vald/internal/io.test]
io/io_test.go:429:28: not enough arguments in call to test.checkFunc
	have (want, io.Writer, error)
	want (want, io.Writer, string, error)

@kevindiu
Copy link
Contributor

kevindiu commented Jul 9, 2020

Also after I ran the go test command in internal folder, the go.sum file changed. Should we fix it also?

git diff ../go.sum
diff --git a/go.sum b/go.sum
index 47181348..6955a0c1 100644
--- a/go.sum
+++ b/go.sum
@@ -832,6 +832,7 @@ golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxb
 golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 h1:SvFZT6jyqRaOeXpc5h/JSfZenJ2O330aBsf7JfSUXmQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
+golang.org/x/time v0.0.0-20191024005414-555d28b269f0 h1:/5xXl8Y5W96D+TtHSlonuFqGHIWVuyCkGJLwGh9JJFs=
 golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=

@vankichi
Copy link
Contributor Author

vankichi commented Jul 9, 2020

io/io_test.go:429:28: not enough arguments in call to test.checkFunc

I could not confirm this problem... 😕
and, CI is success.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 9, 2020

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

@kevindiu
Copy link
Contributor

kevindiu commented Jul 9, 2020

LGTM

kevindiu
kevindiu previously approved these changes Jul 9, 2020
go.sum Outdated Show resolved Hide resolved
hlts2
hlts2 previously approved these changes Jul 9, 2020
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

kevindiu
kevindiu previously approved these changes Jul 9, 2020
@vankichi
Copy link
Contributor Author

vankichi commented Jul 9, 2020

/rebase
/format
/approve

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 9, 2020

[REBASE] Rebase triggered by vankichi for branch: test/internal/fix-failed-internal-test

vankichi added 3 commits July 9, 2020 06:33
Signed-off-by: vankichi <[email protected]>
@vdaas-ci vdaas-ci dismissed stale reviews from kevindiu and hlts2 via a96132e July 9, 2020 06:33
@vdaas-ci vdaas-ci force-pushed the test/internal/fix-failed-internal-test branch from a8e79e0 to a96132e Compare July 9, 2020 06:33
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 9, 2020

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

Copy link
Collaborator

@vdaas-ci vdaas-ci left a comment

Choose a reason for hiding this comment

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

[APPROVED] This PR is approved by vankichi.

@vankichi vankichi merged commit 45a2381 into master Jul 9, 2020
@vankichi vankichi deleted the test/internal/fix-failed-internal-test branch July 9, 2020 07:04
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.

4 participants