Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
test: elevate common http sequences to common#32766
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
There are several instances of data download sequences in test cases. Defined an abstraction for it in common module for easy consumption.
HarshithaKP commented Apr 10, 2020
There are many test cases that will benefit from this, but I changed only one test case now, so that any feedback can be incroporated. |
addaleax commented Apr 10, 2020
@HarshithaKP Did you see #31968? That PR does something similar, and there is some relevant discussion in it. I would not land this unless the people who object to it are okay with this PR. Also, the particular example here doesn’t follow best practices – when a stream body is consumed by concatenating strings, you should always use |
HarshithaKP commented Apr 11, 2020
Thanks @addaleax, I did not know about that PR.
|
Trott left a comment
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.
If we add this, it will need to be documented in test/common/README.md.
receive() is an insufficiently descriptive name for the function if we're going to add it to common.
Trott commented Apr 13, 2020
I don't want to seem like I'm moving the goalposts, so I'll add that I have other reasons I'm hesitant to land stuff like this. Those two things above are the ones I don't think anyone would disagree with. However, in general, I don't think we should be adding things to |
HarshithaKP commented Apr 13, 2020
Closing based on feedback that add things to common only when the case is overwhelming. |
There are several instances of data download sequences in test
cases. Defined an abstraction for it in common module for easy
consumption.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes