mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 69096: Moved a few allocator test helpers to `tests/allocator.hpp`.
Date Wed, 05 Dec 2018 17:36:12 GMT


> On Oct. 24, 2018, 5:22 p.m., Benjamin Mahler wrote:
> > src/tests/allocator.hpp
> > Lines 40-45 (patched)
> > <https://reviews.apache.org/r/69096/diff/1/?file=2101109#file2101109line40>
> >
> >     I think we some other create helpers lying around, e.g. createTask. Is this
where these belong?
> >     
> >     Do you think we need them?
> 
> Meng Zhu wrote:
>     I do not have use cases of other helpers in mind atm. Thus I prefer to pull in only
these two.
> 
> Benjamin Mahler wrote:
>     Do we even need these helpers? They seem pretty trivial, and I wonder how much code
it's actually simplifying. I'm fine with the patch, but it feels heavy to have a new header
and compliation unit just for these, especially since they're only used in two files and are
trivial.

Yeah, we may do these two inline or just copy them around. But I expect that the allocator
tests and benchmarks would share more utilities in the future (after all, they are all allocator
tests). So might as well set up the room for it. If it makes the code structure cleaner, I
think it is worth it.


- Meng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69096/#review210010
-----------------------------------------------------------


On Oct. 19, 2018, 6:28 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69096/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2018, 6:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helps to share the helpers between
> `hierarchical_allocator_tests.cpp` and
> `hierarchical_allocator_benchmarks.cpp`.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 
>   src/tests/CMakeLists.txt 00dbdee1c06e7571fa799850cdaf7f2ccadc8bea 
>   src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
>   src/tests/allocator.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 27fbd9cf0c4442e7675362a806d35bad141ffb6d

> 
> 
> Diff: https://reviews.apache.org/r/69096/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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