From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id CE88942651; Wed, 27 Sep 2023 11:31:24 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8CD4A4026C; Wed, 27 Sep 2023 11:31:24 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 197BD4026B for ; Wed, 27 Sep 2023 11:31:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695807082; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EH6jm8GzEnGaAU9M1LhIO6jjy2Pd4aGPscSrFZc5C2A=; b=QDThgi9SzcUwlPye5YI6D8Icx18ohsw7rK/Aj02HHrdZ4qy3Ev0VVZDvpaFCyT1ODdpMgk A0aSQXP0n/R5lvpzMm7tu7HK/Dz6XM2CQwx5H6xOKLnHUHebkc6Ual6DoIzdxfFJUlJ3xT LRqsOXA0Ds/weyBx9/fH5ig17xubXlw= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-591-CjppHbE2PReP0IfvEHrS7g-1; Wed, 27 Sep 2023 05:31:21 -0400 X-MC-Unique: CjppHbE2PReP0IfvEHrS7g-1 Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2bffd454256so162468021fa.1 for ; Wed, 27 Sep 2023 02:31:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695807080; x=1696411880; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=EH6jm8GzEnGaAU9M1LhIO6jjy2Pd4aGPscSrFZc5C2A=; b=qOuphLCbV7qIh7nV+kX3x3ZbL25a+noiZMLs8KkouKm9frWoO65FSVswZHF3JhE1G+ YbULYE1+KZYFLs/bkQRtT9sakD1H7Km0mfryK1wciqcQ1OLjG48xvd9BWkzyNONEJLtp KBQnjxMIfuaWbAB0a8frN9mCkJ2EnKWH8G5pWEC2mCHm9v90mXaT1Kd4jpvPbJpYs3I/ cyxEropdkXVH91mZDtalypLJFL8KsxlsaIeNDOAP8JO0kJJUO0+MFn8I9380mbzZbXEi gtvAmQJcOPPA7BtSZB4FBgwGb0J0Cx1UKf8yfuGNNjW50WR4ZK7Sz0TPQkWMobbtb7dy 7ZpQ== X-Gm-Message-State: AOJu0YyEUIuxxXiiXKu7VE4Sj7ZMvJjPAbWCrCO/LG/1qj8BGgL7b/qr 0GSbhl6Gbyu5JN9Vjtezmx0fEetLACNu++HoA9ZHAn2Achc03D417IPHTsDnv+QNrGB4b+1nV0S YVxDn5z8MKpOLrJqFkMg= X-Received: by 2002:a2e:6a17:0:b0:2c0:afd:e7ed with SMTP id f23-20020a2e6a17000000b002c00afde7edmr1534024ljc.10.1695807080003; Wed, 27 Sep 2023 02:31:20 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGxEHmbcbi5OH4LdEibwtvlJH0zwkp2nk+2DrGC3rpDLMX2TMEz416H6Lgb06uNlfqb62hLXVQGllm9CLjsNEE= X-Received: by 2002:a2e:6a17:0:b0:2c0:afd:e7ed with SMTP id f23-20020a2e6a17000000b002c00afde7edmr1534011ljc.10.1695807079612; Wed, 27 Sep 2023 02:31:19 -0700 (PDT) MIME-Version: 1.0 References: <20230915115206.132198-1-bruce.richardson@intel.com> In-Reply-To: From: David Marchand Date: Wed, 27 Sep 2023 11:31:07 +0200 Message-ID: Subject: Re: [PATCH 0/2] add checks for tests not in a suite To: Bruce Richardson , Aaron Conole Cc: dev@dpdk.org, Thomas Monjalon , Ferruh Yigit , Jerin Jacob Kollanukkaran , Akhil Goyal , Maxime Coquelin X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Sep 27, 2023 at 10:27=E2=80=AFAM Bruce Richardson wrote: > On Wed, Sep 27, 2023 at 08:30:05AM +0200, David Marchand wrote: > > On Tue, Sep 26, 2023 at 5:01=E2=80=AFPM Aaron Conole wrote: > > > David Marchand writes: > > > > On Tue, Sep 19, 2023 at 10:36=E2=80=AFAM Bruce Richardson > > > > wrote: > > > >> > > To help ensure that we don't have "orphaned" tests not in any = test > > > >> > > suites we can add the following checks: > > > >> > > > > > >> > > * In developer-mode builds, emit a warning for each test defin= ed using > > > >> > > REGISTER_TEST_COMMAND > > > >> > > * In checkpatches, add a check to prevent the addition of new = tests > > > >> > > using the REGISTER_TEST_COMMAND macro > > > >> > > > > > >> > > Bruce Richardson (2): > > > >> > > app/test: emit warning for tests not in a test suite > > > >> > > devtools: check for tests added without a test suite > > > >> > > > > > >> > > app/test/suites/meson.build | 13 ++++++++++++- > > > >> > > buildtools/get-test-suites.py | 12 +++++++++--- > > > >> > > devtools/checkpatches.sh | 8 ++++++++ > > > >> > > 3 files changed, 29 insertions(+), 4 deletions(-) > > > >> > > > > >> > The "non_suite_tests" testsuite returned by > > > >> > buildtools/get-test-suites.py is a bit strange, as it is not a > > > >> > testsuite from meson pov. > > > >> > > > >> Yeah, it is a bit strange, and I'm open to new ideas on other solu= tions. I > > > >> did it that way to avoid having yet another script to scan the fil= es - I > > > >> figured it was faster (in terms of runtime, not dev time) to do th= e > > > > > > > > I had figured it was "faster dev time" that won :-). > > > > I am fine with it, I don't expect more complications in this area i= n the future. > > > > > > > > > > > >> scanning when the files are already being opened and processed by = this one. > > > >> > > > >> Of course, if we can get the un-suitened [:-)] test cases down to = zero, we > > > >> can theoretically drop this check in future, and just use the che= ckpatch > > > >> one. > > > > > > > > Well, that's still a question that nobody seems to comment on. > > > > > > > > What should we do with tests that don't enter one of those testsuit= es, > > > > and are not invoked by the CI? > > > > > > > > Though we may be removing some level of coverage, I am for > > > > cleaning/unused dead code. > > > > > > I guess it does require actually looking at these tests and classifyi= ng > > > them to either put them into the proper suites. As of now, we aren't > > > really removing coverage if they aren't being run - but are any > > > maintainers or developers actually running them? > > > > Could we go a step further than Bruce runtime warning (which is at the > > meson level and does not impact running the test)? > > Perhaps have those orphaned tests fail unless their test names are > > provided in a env variable like > > DPDK_TRUST_ME_I_WILL_SUBMIT_A_PATCH_FOR_THIS_TEST (naming is hard > > ;-))? > > > > With a systematic failure, there is less chance that > > developers/maintainers miss the situation. > > If those developers/maintainers simply waive the warning with the env > > variable and don't send a patch, well.. too bad. > > > > After a release or two, if we don't hear from anyone, we can start > > removing the unused one. > > > I think that seems a littel severe at this point. > > The one gap we have right now, as far as I can see, is actually explainin= g > what the various suite types are, so that developers can choose the right > one for their test(s). We may even need a couple more if some tests do n= ot > fit into the existing categories. Once that is done, we should then start > looking for tests that are obsolete and can be removed. > > If we do already have documentation on the various suites and how to use > them, apologies for my ignorance, and perhaps someone could post a link > here. We have some guidelines but I agree the testsuites are not described. https://doc.dpdk.org/guides/contributing/unit_test.html --=20 David Marchand