-
Notifications
You must be signed in to change notification settings - Fork 454
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: e2e test support http body check #733
feat: e2e test support http body check #733
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #733 +/- ##
==========================================
+ Coverage 33.66% 38.02% +4.35%
==========================================
Files 78 61 -17
Lines 11204 10323 -881
==========================================
+ Hits 3772 3925 +153
+ Misses 7142 6098 -1044
- Partials 290 300 +10 |
cc @2456868764 |
eec9f1a
to
234aee0
Compare
err = json.Unmarshal(body, cReq) | ||
if err != nil { | ||
return nil, nil, fmt.Errorf("unexpected error reading response: %w", err) | ||
if request.Method == "GET" { |
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.
这里建议是否 GET / POST 都返回同样 CapturedRequest 内容结构,如果 为POST, cReq 里body为空。这里要保持一致,而不是要区分不同method, method为POST时也可以做比如 Header等判断
我看了这个PR,主要在于支持POST请求时,请求返回 cReq 只包含 content-type 和 返回body, 而GET请求返回 cReq 包含 Path, Host,Method, Headers 等内容, 这样造成 POST 只能用于 body和cotent-type 断言,不能包含其他比如 Headers, Path, Host等断言,我建议 POST请求也要支持GET请求里包含内容。 |
return nil, nil, fmt.Errorf("unexpected error reading response: %w", err) | ||
} | ||
} | ||
} else if request.Method == "POST" { |
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.
e2e框架不应该根据Method做特殊判断,增加对于rawbody,rawheady的校验即可
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.
好的
}, | ||
}, | ||
Response: http.AssertionResponse{ | ||
ExpectedResponse: http.Response{ |
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.
比如实现一个 ExpectedRawResponse
总结:backend服务增加了可选为 至于测试后续的比较过程 但是以下情况应该是需要增加二选一判断的(不区分GET或POST):
因此我计划的改动为:根据 以上最大的问题在于,测试人员可能在 |
@Uncle-Justice 不是的 echo sever会将请求body塞在响应的json中,依然可以用于校验网关是否对request body做了预期的修改,请求body的校验只能基于echo server。相比之下echo sever是个更通用的测试后端,而echo body目前只是用于插件需要特定的响应body来触发特定测试case。 |
详细沟通后总结:
|
LGTM |
Ⅰ. Describe what this PR did
ExpectedRequest
对body
的断言ExpectedResponse
对body
的断言AssertionMeta.CompareTarget
字段,即测试目标,可设定为Request
或Response
,默认为Request
模式。Request
模式下,相比于以前,只是增加了ExpectedRequest body的校验,其他均未做改动,但是不能校验ExpectedResponse bodyResponse
模式下,无法对request做断言Ⅱ. Does this pull request fix one issue?
fixes #551
fixes #724
Ⅲ. Why don't you add test cases (unit test/integration test)?
Request
和Response
下校验text/plain类型的bodyRequest
和Response
下校验json类型的bodyⅣ. Describe how to verify it
运行所有的e2e测试
Ⅴ. Special notes for reviews
之前的代码逻辑是,可以同时对
ExpectedResponse.Header
和ExpectedRequest.Header
进行断言,目前默认情况下仍保留该逻辑。