> 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.
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. - Benno ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69064/#review211377 ----------------------------------------------------------- On Dec. 18, 2018, 6:41 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69064/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2018, 6:41 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/5/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >