From reviews-return-36741-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Wed Jun 15 09:01:12 2016 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 3264D197DE for ; Wed, 15 Jun 2016 09:01:12 +0000 (UTC) Received: (qmail 48584 invoked by uid 500); 15 Jun 2016 09:01:12 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 48554 invoked by uid 500); 15 Jun 2016 09:01:12 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 48536 invoked by uid 99); 15 Jun 2016 09:01:11 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 15 Jun 2016 09:01:11 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id C9D891C0569; Wed, 15 Jun 2016 09:01:10 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============1017735771665078903==" MIME-Version: 1.0 Subject: Re: Review Request 47374: Separated mesos test helpers into a separate library. From: Till Toenshoff To: Adam B , Kapil Arya , Jie Yu , Artem Harutyunyan , Till Toenshoff , Jan Schlicht Cc: Mesos ReviewBot , Joseph Wu , mesos Date: Wed, 15 Jun 2016 09:01:10 -0000 Message-ID: <20160615090110.30690.2049@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Till Toenshoff X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/47374/ X-Sender: Till Toenshoff References: <20160614002603.30691.86359@reviews.apache.org> In-Reply-To: <20160614002603.30691.86359@reviews.apache.org> Reply-To: Till Toenshoff X-ReviewRequest-Repository: mesos --===============1017735771665078903== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On June 14, 2016, 12:26 a.m., Till Toenshoff wrote: > > Overall, I am not convinced this is reaching the goals we have just yet... > > > > Let me try to specify our goals; > > (a) we want to enable mesos-modules (tests) to reuse our test helpers/utils - things like `cluster.cpp` etc. > > (b) we want to make sure that everything used within the modules is actually publicly exposed to prevent creating dependencies on internal stuff that is not going through deprecations > > (c) we try to be least disruptive on mesos > > > > a. reached > > b. not reached as we do not expose the headers - a module test using that library will still have to tap into the non public mesos folders to get to the needed headers > > c. reached - this patch only changes a makefile > > > > So (b) is the culprit and solving it properly will likely require us to do some serious refactoring of those headers (e.g. `src/tests/mesos.hpp`) to have a clean cut between stuff we want to expose and things we can keep internal to mesos. > > > > Having public test helpers available for module developers is great and very much needed - but I also believe that it might need much more work. I would suggest we create a JIRA for a refactoring of the headers for the test-helpers which would make sure that we have a minimal, stable and clean cut of what we need vs. what we want to keep internal to Mesos. Sounds good? - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47374/#review137245 ----------------------------------------------------------- On June 11, 2016, 12:03 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47374/ > ----------------------------------------------------------- > > (Updated June 11, 2016, 12:03 a.m.) > > > Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, Kapil Arya, Jan Schlicht, and Till Toenshoff. > > > Repository: mesos > > > Description > ------- > > This gives external projects easier access to the test helpers used in > mesos tests. > > For example, a module writer may want to write a test like > `src/tests/oversubscription_tests.cpp`. To build and link against > this library, the module writer would mimic the build flags for tests: > ``` > # Main test file is taken directly from Mesos. > my_module_tests_SOURCES = \ > $(MESOS)/src/tests/main.cpp > > my_module_tests_CPPFLAGS = \ > -I$(GMOCK)/include \ > -I$(GTEST)/include \ > -I$(MESOS)/include/mesos \ > -I$(ZOOKEEPER)/include \ > -I$(ZOOKEEPER)/generated \ > $(AM_CPPFLAGS) > > my_module_tests_LDADD = \ > $(MESOS)/3rdparty/.libs/libgmock.la \ > $(MESOS)/src/.libs/libmesos.la \ > $(MESOS)/src/.libs/libmesos_tests.la > ``` > > > Diffs > ----- > > src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 > > Diff: https://reviews.apache.org/r/47374/diff/ > > > Testing > ------- > > make check on OSX, CentOS 7, Ubuntu 14 > > > Thanks, > > Joseph Wu > > --===============1017735771665078903==--