Refactoring and Add test code for `compress` #622

hlts2 · 2020-08-11T10:05:06Z

Description:

I refactored the compress package because I can't write tests in existing implementations.
For example, we are calling the object directly as follows.
https://github.com/vdaas/vald/blob/master/internal/compress/gzip.go#L71-L75

To solve the problem, I implemented it so that we can mock it and test it.

changes

To be able to mock
Test case of gzip.go

Related Ticket

Flow to LGTM

refactor.
Write a test for gzip.go
Receive LGTM <- if I received LGTM from @vankichi @kevindiu, I will request to author. and
If you get LGTM from the author(8/13~8/20), implement the flow of ↓.
Write tests for new code
Receive LGTM (@kevindiu @vankichi )
merge

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.

pull-assistant · 2020-08-11T10:05:09Z

Best reviewed: commit by commit

Optimal code review plan (7 warnings)

feat: add mock

internal/compress/mock_test.go 93% changes removed in fix: refactor

feat: refactor and and test

...nal/compress/gzip/gzip_mock.go 45% changes removed in feat: add gob/lz4/zs...

internal/compress/gzip.go 60% changes removed in fix: apply suggestio...

internal/compress/gzip/gzip.go 54% changes removed in fix: tweak

fix: refactor

internal/compress/gzip_test.go 45% changes removed in fix: apply suggestio...

feat: add comment

internal/compress/gzip.go 50% changes removed in fix: apply suggestio...

fix: apply suggestion

internal/compress/gzip/gzip.go 61% changes removed in fix: apply suggestio...

...nal/compress/gzip/gzip_mock.go 50% changes removed in feat: add gob/lz4/zs...

feat: add gob/lz4/zstd package

internal/compress/gob.go 55% changes removed in fix: apply suggestio...

internal/compress/lz4.go 70% changes removed in fix: apply suggestio...

internal/compress/zstd.go 67% changes removed in fix: apply suggestio...

fix: tweak

feat: add comment

fix: apply suggestion

Update internal/compress/gob/gob.go

feat: add option

fix: comment

fix: ci error

fix: apply suggestion

internal/compress/gzip_test.go 92% changes removed in fix: apply suggestio...

fix: apply suggestion

🤖 Update license headers / Format go codes and yaml files

fix: apply suggestion

fix: lint warning

Powered by Pull Assistant. Last update 98300ed ... 6431cfc. Read the comment docs.

vdaas-ci · 2020-08-11T10:05:26Z

[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

internal/compress/gzip_test.go

codecov · 2020-08-11T10:13:46Z

Codecov Report

Merging #622 into master will increase coverage by 0.26%.
The diff coverage is 75.60%.

@@            Coverage Diff             @@
##           master     #622      +/-   ##
==========================================
+ Coverage   14.71%   14.98%   +0.26%     
==========================================
  Files         411      412       +1     
  Lines       21576    21595      +19     
==========================================
+ Hits         3175     3236      +61     
+ Misses      18128    18094      -34     
+ Partials      273      265       -8

Impacted Files	Coverage Δ
internal/compress/gzip_option.go	`84.61% <ø> (ø)`
internal/compress/gob.go	`27.90% <66.66%> (+1.71%)`	⬆️
internal/compress/lz4.go	`36.36% <66.66%> (ø)`
internal/compress/zstd.go	`30.64% <66.66%> (+1.13%)`	⬆️
internal/compress/mock.go	`75.00% <75.00%> (ø)`
internal/compress/gzip.go	`100.00% <100.00%> (+73.01%)`	⬆️
internal/compress/zstd_option.go	`81.81% <100.00%> (ø)`
internal/worker/worker.go	`83.63% <0.00%> (-0.91%)`	⬇️
internal/errgroup/group.go	`94.73% <0.00%> (+1.31%)`	⬆️
... and 1 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 9a90b4b...6431cfc. Read the comment docs.

internal/compress/gzip/gzip.go

kevindiu

LGTM if fixed

internal/compress/gzip/gzip_mock.go

internal/compress/gob/gob_mock.go

internal/compress/gob/gob.go

internal/compress/gob/gob_mock.go

internal/compress/lz4/lz4_mock.go

kevindiu · 2020-08-13T04:54:31Z

/rebase

vdaas-ci · 2020-08-13T04:55:06Z

[REBASE] Rebase triggered by kevindiu for branch: test/internal/compress/gzip

kevindiu

LGTM

internal/compress/lz4/lz4.go

vankichi

LGTM

kevindiu

LGTM

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

Co-authored-by: Kevin Diu <kevin_diu@yahoo.com.hk>

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

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

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

vdaas-ci · 2020-08-24T09:54:02Z

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

vdaas-ci

[APPROVED] This PR is approved by kpango.

hlts2 self-assigned this Aug 11, 2020

github-actions bot added area/internal team/set SET team size/XXL type/test Adding tests priority/low labels Aug 11, 2020

github-actions bot reviewed Aug 11, 2020

View reviewed changes

internal/compress/gzip_test.go Show resolved Hide resolved

hlts2 requested a review from kevindiu August 12, 2020 01:04

kevindiu reviewed Aug 12, 2020

View reviewed changes

internal/compress/gzip/gzip.go Outdated Show resolved Hide resolved

kevindiu reviewed Aug 12, 2020

View reviewed changes

internal/compress/gzip/gzip.go Outdated Show resolved Hide resolved

kevindiu reviewed Aug 12, 2020

View reviewed changes

internal/compress/gzip/gzip_mock.go Outdated Show resolved Hide resolved

github-actions bot reviewed Aug 13, 2020

View reviewed changes

internal/compress/gob/gob_mock.go Outdated Show resolved Hide resolved

internal/compress/gob/gob.go Show resolved Hide resolved

internal/compress/gob/gob_mock.go Show resolved Hide resolved

internal/compress/lz4/lz4_mock.go Outdated Show resolved Hide resolved

hlts2 changed the title ~~Refactoring and Add test code for compress/gzip~~ Refactoring and Add test code for compress Aug 13, 2020

vdaas-ci force-pushed the test/internal/compress/gzip branch from 2339790 to 5054647 Compare August 13, 2020 04:55

kevindiu previously approved these changes Aug 13, 2020

View reviewed changes

hlts2 requested a review from vankichi August 13, 2020 05:10

vankichi reviewed Aug 13, 2020

View reviewed changes

internal/compress/lz4/lz4.go Show resolved Hide resolved

hlts2 dismissed kevindiu’s stale review via 3385646 August 13, 2020 06:27

vankichi previously approved these changes Aug 13, 2020

View reviewed changes

kevindiu previously approved these changes Aug 13, 2020

View reviewed changes

hlts2 changed the title ~~Refactoring and Add test code for compress~~ [WIP] Refactoring and Add test code for compress Aug 13, 2020

hlts2 dismissed stale reviews from kevindiu and vankichi via c25578a August 13, 2020 07:58

hlts2 force-pushed the test/internal/compress/gzip branch from c25578a to 5f397c2 Compare August 13, 2020 08:26

hlts2 and others added 17 commits August 24, 2020 09:53

feat: refactor and and test

ed0ddb4

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: refactor

c297a16

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

feat: add comment

75e795a

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: apply suggestion

8c2d639

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

feat: add gob/lz4/zstd package

60ba1fe

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: tweak

7eb9de7

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

feat: add comment

80aea37

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: apply suggestion

ec53972

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

Update internal/compress/gob/gob.go

6d64c21

Co-authored-by: Kevin Diu <kevin_diu@yahoo.com.hk>

feat: add option

b442b89

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: comment

ad11b22

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: ci error

210362d

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: apply suggestion

8c4120e

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: apply suggestion

de7faea

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

🤖 Update license headers / Format go codes and yaml files

f270b29

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

fix: apply suggestion

22c5e52

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

fix: lint warning

6431cfc

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

vdaas-ci dismissed their stale review via 6431cfc August 24, 2020 09:53

vdaas-ci force-pushed the test/internal/compress/gzip branch from 20961dc to 6431cfc Compare August 24, 2020 09:53

vdaas-ci approved these changes Aug 24, 2020

View reviewed changes

kpango merged commit 1d02c4e into master Aug 24, 2020

kpango deleted the test/internal/compress/gzip branch August 24, 2020 10:12

vdaas-ci mentioned this pull request Sep 2, 2020

[patch] update deps #660

Merged

18 tasks

hlts2 mentioned this pull request Sep 7, 2020

Add test for s3/writer and Refactor. #672

Merged

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring and Add test code for `compress` #622

Refactoring and Add test code for `compress` #622

hlts2 commented Aug 11, 2020 •

edited

Loading

pull-assistant bot commented Aug 11, 2020 •

edited

Loading

vdaas-ci commented Aug 11, 2020

codecov bot commented Aug 11, 2020 •

edited

Loading

kevindiu left a comment

kevindiu commented Aug 13, 2020

vdaas-ci commented Aug 13, 2020

kevindiu left a comment

vankichi left a comment

kevindiu left a comment

vdaas-ci commented Aug 24, 2020

vdaas-ci left a comment

Refactoring and Add test code for compress #622

Refactoring and Add test code for compress #622

Conversation

hlts2 commented Aug 11, 2020 • edited Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

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

vdaas-ci commented Aug 11, 2020

codecov bot commented Aug 11, 2020 • edited Loading

Codecov Report

kevindiu left a comment

Choose a reason for hiding this comment

kevindiu commented Aug 13, 2020

vdaas-ci commented Aug 13, 2020

kevindiu left a comment

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

kevindiu left a comment

Choose a reason for hiding this comment

vdaas-ci commented Aug 24, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

Refactoring and Add test code for `compress` #622

Refactoring and Add test code for `compress` #622

hlts2 commented Aug 11, 2020 •

edited

Loading

pull-assistant bot commented Aug 11, 2020 •

edited

Loading

codecov bot commented Aug 11, 2020 •

edited

Loading