From patchwork Tue May 12 17:28:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stuart Haslam X-Patchwork-Id: 48399 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f197.google.com (mail-lb0-f197.google.com [209.85.217.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 9E36A2121F for ; Tue, 12 May 2015 17:29:11 +0000 (UTC) Received: by lbbrr5 with SMTP id rr5sf3432653lbb.3 for ; Tue, 12 May 2015 10:29:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:delivered-to:date:from:to :message-id:references:mime-version:content-disposition:in-reply-to :user-agent:cc:subject:precedence:list-id:list-unsubscribe :list-archive:list-post:list-help:list-subscribe:content-type :content-transfer-encoding:errors-to:sender:x-original-sender :x-original-authentication-results:mailing-list; bh=csfnL3JzMlkXb30vM5iN/rFseEnql1Y3PqcvJ3EfUuE=; b=ldfm2++CqjXaEj95T27HFB/3+mdZ6QRPPFUdmpTVSREiLieQQ/B8CHytMpuX+BmcJl 0ocoU0pFOkOrPyy2qkSgCNzKlwfwaMjmLjNPq326hIWpB5Fya/46S8ALQHEhe6dIMDFW 6Tep+4e6k6RRiovdUz5JqNOnRa+2UqrHUBxSAMhh691i+8wWUiwcm+E6+bpOaNMJIdIt JFaIA7ELGKPCU2SYyUEcyHNBQLoiC7UuLXn/OZCtO7YAHbJi1LbqO4UeGQmmKdAMadrK MFqJRhD4hQENbiSO44M+EjnSDw0lq67bOXB+ZTETE7gUa6RL81FhH6kzPyVDd79+PEV6 /LvA== X-Gm-Message-State: ALoCoQnO2Qxb6gAFSZD7fmgz13pVshJMRebqYuU7ecYgvimTGoSnCOXz8/Vt01JS09R612lITc8j X-Received: by 10.180.98.130 with SMTP id ei2mr2474861wib.0.1431451750132; Tue, 12 May 2015 10:29:10 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.178.195 with SMTP id da3ls66151lac.43.gmail; Tue, 12 May 2015 10:29:10 -0700 (PDT) X-Received: by 10.152.121.66 with SMTP id li2mr12927107lab.65.1431451749960; Tue, 12 May 2015 10:29:09 -0700 (PDT) Received: from mail-lb0-f171.google.com (mail-lb0-f171.google.com. [209.85.217.171]) by mx.google.com with ESMTPS id sa9si10735025lbb.132.2015.05.12.10.29.09 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 May 2015 10:29:09 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.171 as permitted sender) client-ip=209.85.217.171; Received: by lbcga7 with SMTP id ga7so11383542lbc.1 for ; Tue, 12 May 2015 10:29:09 -0700 (PDT) X-Received: by 10.152.87.204 with SMTP id ba12mr13114703lab.35.1431451749808; Tue, 12 May 2015 10:29:09 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp53522lbb; Tue, 12 May 2015 10:29:08 -0700 (PDT) X-Received: by 10.140.86.146 with SMTP id p18mr21620034qgd.49.1431451742984; Tue, 12 May 2015 10:29:02 -0700 (PDT) Received: from lists.linaro.org (lists.linaro.org. [54.225.227.206]) by mx.google.com with ESMTP id r17si16755737qha.109.2015.05.12.10.29.02; Tue, 12 May 2015 10:29:02 -0700 (PDT) Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; Received: by lists.linaro.org (Postfix, from userid 109) id 1156361E47; Tue, 12 May 2015 17:29:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on ip-10-142-244-252.ec2.internal X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, URIBL_BLOCKED autolearn=disabled version=3.4.0 Received: from ip-10-142-244-252.ec2.internal (localhost [127.0.0.1]) by lists.linaro.org (Postfix) with ESMTP id CDAE761E28; Tue, 12 May 2015 17:28:59 +0000 (UTC) X-Original-To: lng-odp@lists.linaro.org Delivered-To: lng-odp@lists.linaro.org Received: by lists.linaro.org (Postfix, from userid 109) id 5FC2461E43; Tue, 12 May 2015 17:28:58 +0000 (UTC) Received: from mail-wi0-f173.google.com (mail-wi0-f173.google.com [209.85.212.173]) by lists.linaro.org (Postfix) with ESMTPS id 2C23461E28 for ; Tue, 12 May 2015 17:28:57 +0000 (UTC) Received: by widdi4 with SMTP id di4so164049959wid.0 for ; Tue, 12 May 2015 10:28:56 -0700 (PDT) X-Received: by 10.180.101.3 with SMTP id fc3mr7003709wib.47.1431451736249; Tue, 12 May 2015 10:28:56 -0700 (PDT) Received: from localhost ([2001:41d0:a:3cb4::1]) by mx.google.com with ESMTPSA id fs9sm28727454wjc.34.2015.05.12.10.28.54 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 12 May 2015 10:28:55 -0700 (PDT) Date: Tue, 12 May 2015 18:28:53 +0100 From: Stuart Haslam To: Christophe Milard Message-ID: <20150512172853.GA29981@localhost> References: <1431341307-9933-1-git-send-email-christophe.milard@linaro.org> <20150511182336.GA18310@localhost> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Topics: patch Cc: LNG ODP Mailman List Subject: Re: [lng-odp] [PATCH 0/3] validation: changing module random X-BeenThere: lng-odp@lists.linaro.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Errors-To: lng-odp-bounces@lists.linaro.org Sender: "lng-odp" X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: stuart.haslam@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.171 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 On Tue, May 12, 2015 at 09:02:18AM +0200, Christophe Milard wrote: > On 11 May 2015 at 20:23, Stuart Haslam wrote: > > > On Mon, May 11, 2015 at 12:48:24PM +0200, Christophe Milard wrote: > > > Hopefully according to the agreement of last week. > > > The changes are applyed to module random, one of the simplest. > > > Please review carefully as all modules will go through the same changes > > > when this goes through... > > > After this patch, at this stage, the tests are still ran from the > > > validation side so as to keep the make check as usual. > > > The plan is to call all the executable from the platform side in one go > > > in a final patch, when all the modules are ready. > > > > > > Christophe Milard (3): > > > validation: preparing for main in tests > > > validation: own main and renaming in odp_random.c > > > validation: creating own dir and lib for random > > > > > > > This patch set is a bit confusing, in patch 2 you make changes in > > preparation for something else that may be done "one day" but then go > > ahead and do it in patch 3. I would rather just see the end result. > > > > No, I don't go ahead: I do create the lib, indeed, but none of the renamed > things (except the main program) actually get exported in the lib (they > remain static). Understood, but you added some code in patch 2 which I was going to comment on before noticing that it was removed in patch 3. > I still think it is important with the renaming to achieve > naming uniformity for clearer tests and future-proofness in case we do have > to export more in the lib. > Having a lib containing just the main gives a chance for platform which > needs it to do the test setup in C before calling the test, from the lib, > in C. (thus allowing for both script and/or C platform side test setup) I agree, and this sort of thing should be in the commit message to explaining why the change is needed rather that just what is being changed. > > Apart from that, the series isn't actually *fixing* anything, so the > > patch descriptions say what you're changing but not why, and without the > > context of future similar changes this just looks like churn. I know > > you've picked the simplest module to start with, but I think it would be > > better to start out with a module that has a problem which this > > restructuring fixes (e.g. pktio), then go from there applying the same > > structure to other modules. > > > > > I see your point here, but I don't know what to do: to actually fix > something, like pktio, the test will have to be ran from the platform side > (that is when we can actually clean-up the pktio things in validation): But > then, during the transition phase, the "make check" results would be split > in 2 (results from tests running from platform side vs results from those > running the old way, from validation side). So a complete patch for pktio > (running from the platform side) would probably be rejected because it > would make the "make check" result more confusing. Based on what we discussed last week I was expecting a first patch that simply moves the TESTS definition from test/validation/Makefile.am and into platform//test/Makefile.am (something like, but not exactly, the diff below). Is this not what you had in mind? > And sending a series of patch taking all modules in one go does not feel > right either if we haven't agreed on one. Hence this patch. Given that this is a restructure with the promise of making future changes easier/possible (these changes alone don't fix anything), and it's intended to be applied to all modules, I think agreeing on the changes being applied to one module is basically the same as agreeing to them for all modules. > So I really wonder whether the approach taken here (agreeing on one module > before taking all other, one by one, and eventually run from the platform > side -all in one go to avoid make check result confusion-) isn't the best... > Do you really think that having a pktio patch and split "make check" > results would have more change to be accepted > > And thanks for taking the time to look at it!! > > Christophe. diff --git a/platform/linux-generic/test/Makefile.am b/platform/linux-generic/test/Makefile.am index 91e361c..a54ccf1 100644 --- a/platform/linux-generic/test/Makefile.am +++ b/platform/linux-generic/test/Makefile.am @@ -1 +1,23 @@ dist_bin_SCRIPTS = $(srcdir)/pktio_env + +COMMON_TEST_DIR=../../../test/validation + +TESTS = $(COMMON_TEST_DIR)/odp_buffer \ + $(COMMON_TEST_DIR)/odp_classification \ + $(COMMON_TEST_DIR)/odp_cpumask \ + $(COMMON_TEST_DIR)/odp_crypto \ + $(COMMON_TEST_DIR)/odp_init \ + $(COMMON_TEST_DIR)/odp_init_abort \ + $(COMMON_TEST_DIR)/odp_init_log \ + $(COMMON_TEST_DIR)/odp_packet \ + odp_pktio_run \ + $(COMMON_TEST_DIR)/odp_pool \ + $(COMMON_TEST_DIR)/odp_queue \ + $(COMMON_TEST_DIR)/odp_random \ + $(COMMON_TEST_DIR)/odp_scheduler \ + $(COMMON_TEST_DIR)/odp_shared_memory \ + $(COMMON_TEST_DIR)/odp_synchronizers \ + $(COMMON_TEST_DIR)/odp_time \ + $(COMMON_TEST_DIR)/odp_timer \ + $(COMMON_TEST_DIR)/odp_thread \ + $(COMMON_TEST_DIR)/odp_ver_abt_log_dbg diff --git a/test/validation/odp_pktio_run b/platform/linux-generic/test/odp_pktio_run similarity index 100% rename from test/validation/odp_pktio_run rename to platform/linux-generic/test/odp_pktio_run diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am index 7470d9d..0962c3e 100644 --- a/test/validation/Makefile.am +++ b/test/validation/Makefile.am @@ -13,6 +13,7 @@ EXECUTABLES = odp_buffer \ odp_init_abort \ odp_init_log \ odp_packet \ + odp_pktio \ odp_pool \ odp_queue \ odp_random \ @@ -24,17 +25,7 @@ EXECUTABLES = odp_buffer \ odp_thread \ odp_ver_abt_log_dbg -COMPILE_ONLY = odp_pktio - -TESTSCRIPTS = odp_pktio_run - -if test_vald -TESTS = $(EXECUTABLES) $(TESTSCRIPTS) -endif - -dist_bin_SCRIPTS = odp_pktio_run - -bin_PROGRAMS = $(EXECUTABLES) $(COMPILE_ONLY) +bin_PROGRAMS = $(EXECUTABLES) ODP_CU_COMMON=common/odp_cunit_common.c