> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote: > > src/tests/master_load_tests.cpp > > Lines 555 (patched) > > <https://reviews.apache.org/r/69064/diff/4/?file=2113358#file2113358line555> > > > > Can you use some other header to test this case, so that we don't need > > to disable the test? You could even set an arbitrary header which is not > > interpreted by libprocess. > > Benno Evers wrote: > The endpoints themselves will ignore all headers, so the idea of this > test was to verify that libprocess can still correctly handle the headers > that libprocess cares about. So using another header which is ignored by > libprocess will not really work for that purpose. > > I suppose we could drop the test completely, but I think the chances are > pretty low that we will remember (and allocate time) to write this test from > scratch in the future, when it becomes possible to enable it. > > Greg Mann wrote: > Could you elaborate on the reason for the test? I don't quite understand > why we need a test to verify that libprocess can still correctly handle some > headers? I was thinking that the test was trying to verify that > de-duplication was working correctly in cases where requests differ only in > the headers that they specify, but it doesn't look like that's what the body > of the test is doing. > > Benno Evers wrote: > Let me try to elaborate: > > Right now, we get a different response when requesting a gzipped vs. a > plain text response: (with 'response' I'm referring to the data that's sent > over the wire, not the decoded content, which also answers your question > below): > > ``` > $ echo -e "GET /state HTTP/1.1\nAccept-Encoding: gzip\r\n" | nc localhost > 5050 | xxd > 00000000: 4854 5450 2f31 2e31 2032 3030 204f 4b0d HTTP/1.1 200 OK. > 00000010: 0a43 6f6e 7465 6e74 2d45 6e63 6f64 696e .Content-Encodin > 00000020: 673a 2067 7a69 700d 0a44 6174 653a 2057 g: gzip..Date: W > 00000030: 6564 2c20 3139 2044 6563 2032 3031 3820 ed, 19 Dec 2018 > 00000040: 3130 3a30 323a 3433 2047 4d54 0d0a 436f 10:02:43 GMT..Co > 00000050: 6e74 656e 742d 5479 7065 3a20 6170 706c ntent-Type: appl > 00000060: 6963 6174 696f 6e2f 6a73 6f6e 0d0a 436f ication/json..Co > 00000070: 6e74 656e 742d 4c65 6e67 7468 3a20 3739 ntent-Length: 79 > 00000080: 330d 0a0d 0a1f 8b08 0000 0000 0000 03ad 3............... > 00000090: 55cb 8ed3 3014 fd15 6489 5d1f 491a cf84 U...0...d.].I... > [...] > > $ echo -e "GET /state HTTP/1.1\nAccept-Encoding: identity\r\n" | nc > localhost 5050 | xxd > 00000000: 4854 5450 2f31 2e31 2032 3030 204f 4b0d HTTP/1.1 200 OK. > 00000010: 0a44 6174 653a 2057 6564 2c20 3139 2044 .Date: Wed, 19 D > 00000020: 6563 2032 3031 3820 3130 3a30 333a 3034 ec 2018 10:03:04 > 00000030: 2047 4d54 0d0a 436f 6e74 656e 742d 5479 GMT..Content-Ty > 00000040: 7065 3a20 6170 706c 6963 6174 696f 6e2f pe: application/ > 00000050: 6a73 6f6e 0d0a 436f 6e74 656e 742d 4c65 json..Content-Le > 00000060: 6e67 7468 3a20 3138 3935 0d0a 0d0a 7b22 ngth: 1895....{" > 00000070: 7665 7273 696f 6e22 3a22 312e 362e 3022 version":"1.6.0" > 00000080: 2c22 6275 696c 645f 6461 7465 223a 2232 ,"build_date":"2 > 00000090: 3031 382d 3035 2d30 3420 3230 3a33 343a 018-05-04 20:34: > [...] > ``` > > If this were to change due to the request being batched together with > another one, we'd probably consider it to be a bug, and the idea of having > this test is to ensure this will not accidentally break in the future. > > Technically, we could also move this test to libprocess and just verify > that two HTTP responses created from the same future will get encoded > independently, but that test would also need to be `DISABLED` right now for > the same reason, i.e. libprocess not being able to actually receive the > gzip-encoded response. > > Also, thinking about it, clients currently can't depend on response > compression anyways due to MESOS-9451/MESOS-9453, so we could probably also > just accept the current state of affairs and drop this test case completely.
Dropping this issue since we removed the test for as per our discussions. - Benno ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69064/#review211377 ----------------------------------------------------------- On Dec. 19, 2018, 6:39 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69064/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2018, 6:39 p.m.) > > > Review request for mesos, Alexander Rukletsov and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > This commit adds a set of unit test to verify that > basic Master HTTP endpoints still work correctly > under the presence of request caching. > > > Diffs > ----- > > src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa > src/tests/master_load_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/69064/diff/6/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
