mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 69064: Added unit tests for Master HTTP endpoints.
Date Tue, 18 Dec 2018 18:42:40 GMT


> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote:
> > src/tests/master_load_tests.cpp
> > Lines 293-297 (patched)
> > <https://reviews.apache.org/r/69064/diff/4/?file=2113358#file2113358line293>
> >
> >     Should we add something like
> >     
> >     ASSERT_TRUE(Clock::now() - whileLoopStartTime < Seconds(15));
> >     
> >     to make sure that we don't end up getting stuck spinning here due to some bug
in the future?

Good point!


> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote:
> > src/tests/master_load_tests.cpp
> > Lines 461 (patched)
> > <https://reviews.apache.org/r/69064/diff/4/?file=2113358#file2113358line461>
> >
> >     Remove trailing space after `EXPECT_TRUE`.

I did, but not without shedding a silent tear for the loss of alignment with the `EXPECT_FALSE()`
below ;)


> 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.

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.


- 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message