Add internal roundtrip test code #589

kevindiu · 2020-07-20T06:36:03Z

Author review required?

Description:

This PR implements the roundtrip test case.
This PR also fix the bug that it did not return errors.ErrTransportRetryable when the roundtrip success and the request is retryable.
Also this PR update the roundtrip logic that making the request terminated if the request is not retryable.

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-07-20T06:36:22Z

Best reviewed: commit by commit

Optimal code review plan (9 warnings)

add test code and comments

...et/http/transport/roundtrip.go 50% changes removed in refactor and add tes...

refactor

...et/http/transport/roundtrip.go 50% changes removed in use retryableStatus

refactor and add test code

...et/http/transport/roundtrip.go 45% changes removed in use retryableStatus

add test case

...et/http/transport/roundtrip.go 88% changes removed in use named return

remove unused code

fix comment

add retry mechanism

...et/http/transport/roundtrip.go 69% changes removed in use named return

rename var

...et/http/transport/roundtrip.go 80% changes removed in use named return

fix

use named return

...et/http/transport/roundtrip.go 67% changes removed in rename var

...tp/transport/roundtrip_test.go 50% changes removed in fix

fix

fix: tweak

...tp/transport/roundtrip_test.go 67% changes removed in fix: apply suggestio...

fix: tweak

fix: apply suggestion

Powered by Pull Assistant. Last update c1045e9 ... 29a2744. Read the comment docs.

vdaas-ci · 2020-07-20T06:36:23Z

[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-07-20T06:45:05Z

Codecov Report

Merging #589 into master will increase coverage by 0.05%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master     #589      +/-   ##
==========================================
+ Coverage   12.13%   12.19%   +0.05%     
==========================================
  Files         409      409              
  Lines       21329    21340      +11     
==========================================
+ Hits         2588     2602      +14     
+ Misses      18456    18454       -2     
+ Partials      285      284       -1

Impacted Files	Coverage Δ
internal/net/http/transport/roundtrip.go	`91.30% <86.66%> (+11.30%)`	⬆️

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 bb35dd9...29a2744. Read the comment docs.

internal/net/http/transport/roundtrip_test.go

kevindiu · 2020-07-21T04:17:10Z

/rebase

vdaas-ci · 2020-07-21T04:17:49Z

[REBASE] Rebase triggered by kevindiu for branch: test/net/http/transport/roundtrip

internal/net/http/transport/roundtrip_test.go

internal/net/http/transport/roundtrip.go

hlts2

LGTM

vankichi

LGTM

internal/net/http/transport/roundtrip.go

vdaas-ci · 2020-08-04T08:54:31Z

[REBASE] Rebase triggered by kevindiu for branch: test/net/http/transport/roundtrip

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

vankichi · 2020-08-04T10:16:03Z

/rebase
/format
/approve

vdaas-ci · 2020-08-04T10:16:41Z

[REBASE] Rebase triggered by vankichi for branch: test/net/http/transport/roundtrip

vdaas-ci · 2020-08-04T10:18:01Z

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

vdaas-ci

[APPROVED] This PR is approved by vankichi.

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

github-actions bot reviewed Jul 20, 2020

View reviewed changes

internal/net/http/transport/roundtrip_test.go Outdated Show resolved Hide resolved

github-actions bot added size/XL and removed size/L labels Jul 20, 2020

github-actions bot reviewed Jul 20, 2020

View reviewed changes

internal/net/http/transport/roundtrip_test.go Outdated Show resolved Hide resolved

internal/net/http/transport/roundtrip_test.go Outdated Show resolved Hide resolved

kevindiu changed the title ~~Add roundtrip test code~~ [WIP] Add roundtrip test code Jul 21, 2020

kevindiu changed the title ~~[WIP] Add roundtrip test code~~ Add internal roundtrip test code Jul 21, 2020

kevindiu requested a review from hlts2 July 21, 2020 04:12

vdaas-ci force-pushed the test/net/http/transport/roundtrip branch from 3fd8c3c to e9fb3c6 Compare July 21, 2020 04:17

github-actions bot reviewed Jul 21, 2020

View reviewed changes

internal/net/http/transport/roundtrip_test.go Show resolved Hide resolved

hlts2 reviewed Jul 21, 2020

View reviewed changes

internal/net/http/transport/roundtrip.go Show resolved Hide resolved

hlts2 previously approved these changes Jul 21, 2020

View reviewed changes

kevindiu marked this pull request as ready for review July 22, 2020 01:01

kevindiu requested a review from vankichi July 22, 2020 01:01

kevindiu dismissed hlts2’s stale review via 9f1adb2 July 22, 2020 02:19

vankichi previously approved these changes Jul 22, 2020

View reviewed changes

kevindiu requested review from kpango, rinx and kmrmt July 22, 2020 05:37

rinx reviewed Jul 27, 2020

View reviewed changes

internal/net/http/transport/roundtrip.go Outdated Show resolved Hide resolved

internal/net/http/transport/roundtrip.go Outdated Show resolved Hide resolved

kpango reviewed Jul 28, 2020

View reviewed changes

internal/net/http/transport/roundtrip.go Outdated Show resolved Hide resolved

kevindiu added 16 commits August 4, 2020 08:54

add test code and comments

c1045e9

refactor

ec381b1

refactor and add test code

41b9dde

add test case

2265bbd

remove unused code

a4c68ca

fix comment

17d9b55

add retry mechanism

db9ceff

rename var

eaadbe7

fix

8c51774

use named return

a6634d4

rename var

b639440

rename

065ce15

rename var

73dc8c3

use retryableStatus

42a6267

add logging message

ef867d4

fix

7f2000e

vdaas-ci force-pushed the test/net/http/transport/roundtrip branch from 20da29c to 7f2000e Compare August 4, 2020 08:54

kevindiu requested a review from hlts2 August 4, 2020 09:25

hlts2 added 3 commits August 4, 2020 19:09

fix: tweak

793ffbd

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

fix: tweak

5a90a7e

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

fix: apply suggestion

29a2744

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

vankichi approved these changes Aug 4, 2020

View reviewed changes

vdaas-ci approved these changes Aug 4, 2020

View reviewed changes

vankichi merged commit 9cd5fc9 into master Aug 5, 2020

vankichi deleted the test/net/http/transport/roundtrip branch August 5, 2020 01:23

vdaas-ci mentioned this pull request Sep 2, 2020

[patch] update deps #660

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add internal roundtrip test code #589

Add internal roundtrip test code #589

kevindiu commented Jul 20, 2020 •

edited

Loading

pull-assistant bot commented Jul 20, 2020 •

edited

Loading

vdaas-ci commented Jul 20, 2020

codecov bot commented Jul 20, 2020 •

edited

Loading

kevindiu commented Jul 21, 2020

vdaas-ci commented Jul 21, 2020

hlts2 left a comment

vankichi left a comment

vdaas-ci commented Aug 4, 2020

vankichi commented Aug 4, 2020

vdaas-ci commented Aug 4, 2020

vdaas-ci commented Aug 4, 2020

vdaas-ci left a comment

Add internal roundtrip test code #589

Add internal roundtrip test code #589

Conversation

kevindiu commented Jul 20, 2020 • edited Loading

Description:

Related Issue:

How Has This Been Tested?:

Environment:

Types of changes:

Changes to Core Features:

Checklist:

pull-assistant bot commented Jul 20, 2020 • edited Loading

vdaas-ci commented Jul 20, 2020

codecov bot commented Jul 20, 2020 • edited Loading

Codecov Report

kevindiu commented Jul 21, 2020

vdaas-ci commented Jul 21, 2020

hlts2 left a comment

Choose a reason for hiding this comment

vankichi left a comment

Choose a reason for hiding this comment

vdaas-ci commented Aug 4, 2020

vankichi commented Aug 4, 2020

vdaas-ci commented Aug 4, 2020

vdaas-ci commented Aug 4, 2020

vdaas-ci left a comment

Choose a reason for hiding this comment

kevindiu commented Jul 20, 2020 •

edited

Loading

pull-assistant bot commented Jul 20, 2020 •

edited

Loading

codecov bot commented Jul 20, 2020 •

edited

Loading