Test for the new caching configuration options (#810) #199
Conversation
There was a problem hiding this comment.
At least these tests are missed:
- Missing functional tests for cache tempesta#810 (comment) max-age and all similar directives for requests and responses. Need to test that they work as expected, i.e. cache entries are retrieved or not depending on the directives
- testing point 3 from Automatic Platform Optimization tempesta#1516 (there are link to error output when Tempesta config is empty. #530)
- Test the header Cache-Control: public, no-cache="Set-Cookie", must-revalidate, max-age=120 and make sure that Set-Cookie isn't stored in the cache. Test other headers in no-cache. Test list of headers in no-cache (test case 3 from Cache behavior tuning tempesta#530)
Ported to "framework". Extended the test suit for additional Cache-Control directives. Added PURGE-GET tests for header removal.
|
Did a complete rework of the test_cache_control suite to use |
There was a problem hiding this comment.
Looks good. Besides the comments, I also didn't find no-transform for either requests or responses and no-cache for requests.
Also I didn't do too deep review since I didn't touch the tests for a while, so I appreciate if @Dmitry-Gouriev also review the PR.
Also removed duplicates of wait_all_connections() and fixed incorrect dmesg string in the start_tempesta().
|
Updated the pull request. Also, there was a small bug in |
There was a problem hiding this comment.
LGTM for me, but there are comments from @Dmitry-Gouriev . Once the comments are resolved, the PR can be merged.
* Fixed wrong check for second response status. * Larger windows for cache lifetimes.
|
Looks fine in general.
Also I wonder how you treat an Authorization: header, however this seems to be in accordance with never HTTP spec RFC 7234 3.2. Besides ResponsePublicCached2. Although your remark
I can not inderstand why this response may be cached. |
|
RFC 7234 only forbids serving responses from the first "Authorization" request to subsequent requests, it does not tell what to do when subsequent requests turn out to have "Authorization" header while the first request had no such header. Talking about windows -- I'd be glad to make those larger, but that would make the tests run even longer. And we already have one test running for 3 minutes. |
I'll simple repeat my response from private discussion. When authorization is required, and Authorization: header is not present, the server normaly responds with 401 and such response may not be cached. If the server responds with 2xx this means that the particular resource is for public access, so can be responded from a cache with Authorization: header and without it. Analysing this case you have convinced me that everything goes OK and my question was unnecessary. The only doubtful case remaining is when access rules change at server side and cached resource becomes for resticted access. I hope, cache cleaning or proxy restart can help in this rare situation. |
|
I found a mention about the case, when people need to cache responses for requests with Authorization header. It seems from the tests involving Authorization header in a request we are compliant with RFC 7234 3.2. Regarding the network delays I think we're good with 1-2 second delays since the test suite already runs for a very long time and we either run it in a local host or in LAN, usually even inside a single physical server, so latency is quite small in our environments. |
No description provided.