-
Notifications
You must be signed in to change notification settings - Fork 528
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
feat: rewrite e2e test(upstream_chash_query_string_arg_xxx_test) with ginkgo #1504
feat: rewrite e2e test(upstream_chash_query_string_arg_xxx_test) with ginkgo #1504
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1504 +/- ##
==========================================
- Coverage 69.01% 68.97% -0.04%
==========================================
Files 48 48
Lines 3056 3056
==========================================
- Hits 2109 2108 -1
- Misses 707 708 +1
Partials 240 240
Continue to review full report at Codecov.
|
assert.Nil(t, err) | ||
resp, err := http.DefaultClient.Do(req) | ||
assert.Nil(t, err) | ||
respBody, err := ioutil.ReadAll(resp.Body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to assert the err
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
req, err := http.NewRequest("GET", url, nil) | ||
assert.Nil(t, err) | ||
resp, err := http.DefaultClient.Do(req) | ||
assert.Nil(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defer closing the resp.Body at here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. It's my negligence. I'll check the previous use cases and correct the same problems
} else { | ||
res[body]++ | ||
} | ||
resp.Body.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
0f787ea
to
1d18070
Compare
1d18070
to
a1709a8
Compare
cc @starsz PTAL, Thks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Please answer these questions before submitting a pull request
Why submit this pull request?
Bugfix
New feature provided
Improve performance
Backport patches
Related issues
Feat: Rewrite all E2E tests using the Ginkgo #1500
New feature or improvement