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 ngt index path of test case #2130

Merged
merged 4 commits into from
Jul 12, 2023
Merged

Conversation

hlts2
Copy link
Collaborator

@hlts2 hlts2 commented Jul 12, 2023

Description:

WHAT

I fixed index path of test case

WHY

The test failed because it was the same index path as the other test cases.

https://github.com/vdaas/vald/blob/main/internal/core/algorithm/ngt/ngt_test.go#L1807-L1832

Related Issue:

Versions:

  • Go Version: 1.20.6
  • Docker Version: 20.10.8
  • Kubernetes Version: v1.27.3
  • NGT Version: 2.0.16

Checklist:

Special notes for your reviewer:

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

@hlts2 hlts2 self-assigned this Jul 12, 2023
@hlts2 hlts2 requested review from a team, kmrmt, kevindiu and kpango and removed request for a team July 12, 2023 02:02
kpango
kpango previously approved these changes Jul 12, 2023
kevindiu
kevindiu previously approved these changes Jul 12, 2023
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.03 ⚠️

Comparison is base (b92e4bc) 31.12% compared to head (428219b) 31.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2130      +/-   ##
==========================================
- Coverage   31.12%   31.10%   -0.03%     
==========================================
  Files         337      339       +2     
  Lines       32778    32831      +53     
==========================================
+ Hits        10202    10211       +9     
- Misses      22128    22167      +39     
- Partials      448      453       +5     
Impacted Files Coverage Δ
hack/benchmark/core/benchmark/strategy/search.go 0.00% <0.00%> (ø)
hack/benchmark/internal/core/algorithm/ngt/ngt.go 0.00% <0.00%> (ø)

... and 5 files with indirect coverage changes

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

@hlts2 hlts2 dismissed stale reviews from kevindiu and kpango via e25266a July 12, 2023 02:41
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 12, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 428219b
Status: ✅  Deploy successful!
Preview URL: https://9f4670eb.vald.pages.dev
Branch Preview URL: https://bugfix-test-agent-ngt-index.vald.pages.dev

View logs

@hlts2 hlts2 requested review from kpango and kevindiu July 12, 2023 02:42
kevindiu
kevindiu previously approved these changes Jul 12, 2023
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

@hlts2 hlts2 merged commit 6f5fb18 into main Jul 12, 2023
@hlts2 hlts2 deleted the bugfix/test/agent-ngt-index-path branch July 12, 2023 07:17
kpango pushed a commit that referenced this pull request Jul 20, 2023
ykadowak pushed a commit that referenced this pull request Jul 25, 2023
* fix index path for test

Signed-off-by: hlts2 <[email protected]>

* use t.TempDir for index path

Signed-off-by: hlts2 <[email protected]>

* create inde temp directory and fix fails test

Signed-off-by: hlts2 <[email protected]>

* refactoring comprator

Signed-off-by: hlts2 <[email protected]>

---------

Signed-off-by: hlts2 <[email protected]>
kpango added a commit that referenced this pull request Aug 31, 2023
@hlts2 hlts2 mentioned this pull request Sep 13, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
ykadowak added a commit that referenced this pull request Nov 30, 2023
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.

5 participants