Add test for `internal/db/storage/blob/s3/reader`. #718

hlts2 · 2020-09-23T03:47:07Z

Signed-off-by: hlts2 [email protected]

Description:

I refactored and added test fors3/reader.go

Refactor

WHY
I am writing test about s3/reader now. But There were some places where I couldn’t write a test.
The reason are as follows.

The current implementation calls New method inside test target function.

vald/internal/db/storage/blob/s3/reader/reader.go

Line 97 in 92f7fff

body, err = ctxio.NewReaderWithContext(ctx, body)
The current implementation use s3 struct object, not interface

vald/internal/db/storage/blob/s3/reader/reader.go

Line 40 in 92f7fff

service *s3.S3

WHAT

I want to create io interface to create object via interafce like comress. https://github.com/vdaas/vald/blob/master/internal/compress/lz4/lz4.go#L49
I want to use s3iface.S3API interface instead of s3 struct(*s3.S3)
https://github.com/aws/aws-sdk-go/blob/v1.34.28/service/s3/s3iface/interface.go#L62-L441

type reader struct {
	service *s3.S3
}
↓
type reader struct {
	service s3iface.S3API
}

Related Issue:

How Has This Been Tested?:

Environment:

Go Version: 1.15.2
Docker Version: 19.03.8
Kubernetes Version: 1.18.2
NGT Version: 1.12.1

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-09-23T03:47:13Z

Best reviewed: commit by commit

Optimal code review plan

feat: add test

feat: add test and mock

feat: add alias for s3

fix: lint warning

fix: apply suggestion

Update internal/db/storage/blob/s3/reader/reader.go

fix: ci error

🤖 Update license headers / Format go codes and yaml files

add pacakge comment

Powered by Pull Assistant. Last update 4b46dd1 ... cc5fbb5. Read the comment docs.

vdaas-ci · 2020-09-23T03:47:21Z

[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 · 2020-09-23T03:53:47Z

Codecov Report

Merging #718 into master will increase coverage by 0.29%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #718      +/-   ##
==========================================
+ Coverage   15.46%   15.76%   +0.29%     
==========================================
  Files         420      421       +1     
  Lines       19531    19536       +5     
==========================================
+ Hits         3021     3079      +58     
+ Misses      16287    16229      -58     
- Partials      223      228       +5

Impacted Files	Coverage Δ
internal/db/storage/blob/s3/reader/option.go	`100.00% <ø> (ø)`
internal/db/storage/blob/s3/reader/reader.go	`67.44% <100.00%> (+67.44%)`	⬆️
internal/db/storage/blob/s3/reader/reader_mock.go	`100.00% <100.00%> (ø)`
internal/worker/queue.go	`95.00% <0.00%> (-5.00%)`	⬇️
internal/errgroup/group.go	`92.42% <0.00%> (-1.52%)`	⬇️
internal/worker/worker.go	`83.33% <0.00%> (-1.05%)`	⬇️

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 4410fa4...f4ec3e9. Read the comment docs.

hlts2 · 2020-09-24T06:43:45Z

/rebase

vdaas-ci · 2020-09-24T06:44:20Z

[REBASE] Rebase triggered by hlts2 for branch: test/internal/add-test-for-s3-reader

internal/db/storage/blob/s3/reader/reader_test.go

internal/db/storage/blob/s3/reader/reader_mock.go

internal/db/storage/blob/s3/reader/reader.go

internal/db/storage/blob/s3/reader/reader_mock.go

internal/db/storage/blob/s3/reader/reader.go

kevindiu

LGTM

kevindiu

please fix CI

kevindiu

LGTM

vankichi · 2020-10-01T05:42:27Z

/format

vdaas-ci · 2020-10-01T05:43:40Z

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

kevindiu

LGTM

vankichi · 2020-10-02T09:25:32Z

/rebase
/format
/approve

vdaas-ci · 2020-10-02T09:26:08Z

[REBASE] Rebase triggered by vankichi for branch: test/internal/add-test-for-s3-reader

vdaas-ci · 2020-10-02T09:27:06Z

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

vdaas-ci

[APPROVED] This PR is approved by vankichi.

vankichi · 2020-10-02T11:59:08Z

/rebase
/format
/approve

vdaas-ci · 2020-10-02T11:59:43Z

[REBASE] Rebase triggered by vankichi for branch: test/internal/add-test-for-s3-reader

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

Co-authored-by: Kevin Diu <[email protected]>

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

Signed-off-by: vdaas-ci <[email protected]>

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

vdaas-ci · 2020-10-02T12:00:41Z

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

vdaas-ci

[APPROVED] This PR is approved by vankichi.

hlts2 self-assigned this Sep 23, 2020

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

github-actions bot added size/XL size/XXL and removed size/L size/XL labels Sep 23, 2020

vdaas-ci force-pushed the test/internal/add-test-for-s3-reader branch from ea64ee7 to 9da7fac Compare September 24, 2020 06:44

hlts2 changed the title ~~[WIP] Add test for internal/db/storage/blob/s3/reader.~~ Add test for internal/db/storage/blob/s3/reader. Sep 24, 2020

github-actions bot reviewed Sep 24, 2020

View reviewed changes

internal/db/storage/blob/s3/reader/reader_test.go Outdated Show resolved Hide resolved

internal/db/storage/blob/s3/reader/reader_test.go Outdated Show resolved Hide resolved

hlts2 requested a review from kevindiu September 24, 2020 07:32

github-actions bot reviewed Sep 24, 2020

View reviewed changes

kevindiu reviewed Sep 24, 2020

View reviewed changes

internal/db/storage/blob/s3/reader/reader.go Outdated Show resolved Hide resolved

internal/db/storage/blob/s3/reader/reader_mock.go Show resolved Hide resolved

kevindiu reviewed Sep 24, 2020

View reviewed changes

internal/db/storage/blob/s3/reader/reader.go Show resolved Hide resolved

kevindiu previously approved these changes Sep 24, 2020

View reviewed changes

kevindiu reviewed Sep 24, 2020

View reviewed changes

hlts2 dismissed kevindiu’s stale review via 613d436 September 24, 2020 09:12

kevindiu previously approved these changes Sep 24, 2020

View reviewed changes

hlts2 requested a review from vankichi September 30, 2020 04:09

vdaas-ci dismissed kevindiu’s stale review via 73725f0 October 1, 2020 05:44

kevindiu previously approved these changes Oct 2, 2020

View reviewed changes

vankichi previously approved these changes Oct 2, 2020

View reviewed changes

vdaas-ci dismissed stale reviews from vankichi and kevindiu via 3ac6c75 October 2, 2020 09:26

vdaas-ci force-pushed the test/internal/add-test-for-s3-reader branch from cc5fbb5 to 3ac6c75 Compare October 2, 2020 09:26

vdaas-ci previously approved these changes Oct 2, 2020

View reviewed changes

hlts2 and others added 13 commits October 2, 2020 11:59

feat: add test

01315dd

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

feat: add test and mock

6515c57

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

feat: add test case

5994cd6

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

feat: add comment

46fdcd8

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

fix: test case

dc8fd5e

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

fix: import path

8c20f16

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

feat: add alias for s3

72546f6

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

fix: lint warning

f9c9836

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

fix: apply suggestion

8f4062d

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

Update internal/db/storage/blob/s3/reader/reader.go

7e5b1f7

Co-authored-by: Kevin Diu <[email protected]>

fix: ci error

62823a7

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

🤖 Update license headers / Format go codes and yaml files

7ccaf20

Signed-off-by: vdaas-ci <[email protected]>

add pacakge comment

f4ec3e9

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

vdaas-ci dismissed their stale review via f4ec3e9 October 2, 2020 11:59

vdaas-ci force-pushed the test/internal/add-test-for-s3-reader branch from 3ac6c75 to f4ec3e9 Compare October 2, 2020 11:59

vdaas-ci approved these changes Oct 2, 2020

View reviewed changes

vankichi merged commit 3caef54 into master Oct 2, 2020

vankichi deleted the test/internal/add-test-for-s3-reader branch October 2, 2020 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test for `internal/db/storage/blob/s3/reader`. #718

Add test for `internal/db/storage/blob/s3/reader`. #718

hlts2 commented Sep 23, 2020 •

edited

Loading

pull-assistant bot commented Sep 23, 2020 •

edited

Loading

vdaas-ci commented Sep 23, 2020

codecov bot commented Sep 23, 2020 •

edited

Loading

hlts2 commented Sep 24, 2020

vdaas-ci commented Sep 24, 2020

kevindiu left a comment

kevindiu left a comment

kevindiu left a comment

vankichi commented Oct 1, 2020

vdaas-ci commented Oct 1, 2020

kevindiu left a comment

vankichi commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci left a comment

vankichi commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci left a comment

Add test for internal/db/storage/blob/s3/reader. #718

Add test for internal/db/storage/blob/s3/reader. #718

Conversation

hlts2 commented Sep 23, 2020 • edited Loading

Description:

Refactor

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

pull-assistant bot commented Sep 23, 2020 • edited Loading

vdaas-ci commented Sep 23, 2020

codecov bot commented Sep 23, 2020 • edited Loading

Codecov Report

hlts2 commented Sep 24, 2020

vdaas-ci commented Sep 24, 2020

kevindiu left a comment

Choose a reason for hiding this comment

kevindiu left a comment

Choose a reason for hiding this comment

kevindiu left a comment

Choose a reason for hiding this comment

vankichi commented Oct 1, 2020

vdaas-ci commented Oct 1, 2020

kevindiu left a comment

Choose a reason for hiding this comment

vankichi commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

vankichi commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci commented Oct 2, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

Add test for `internal/db/storage/blob/s3/reader`. #718

Add test for `internal/db/storage/blob/s3/reader`. #718

hlts2 commented Sep 23, 2020 •

edited

Loading

pull-assistant bot commented Sep 23, 2020 •

edited

Loading

codecov bot commented Sep 23, 2020 •

edited

Loading