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 B0F964264F; Wed, 27 Sep 2023 08:30:22 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3800940296; Wed, 27 Sep 2023 08:30:22 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 2A3E940271 for ; Wed, 27 Sep 2023 08:30:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695796220; 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=HFnib67QhgCaaUoKsZWlp2Vwzgh+j7FgnEi3HnglTX4=; b=e+oVVD+nbMuMX8SOOvOQXPNhJxQ8kRGGGuCEre03Y4nmPozOMy2ZOOZobAC1wXl/am2lnQ un1I8HW+e5LwYTkwjzC45oRyObIDHr6eiRLwAWEODsvjn/BGCVMzoOp+kP9UwSjbAkgYp1 0/TV4CJkzd93cwChMvQ30gkZiPtQ8gw= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-584-z8whISQAPg-NeyCJc0qOrA-1; Wed, 27 Sep 2023 02:30:19 -0400 X-MC-Unique: z8whISQAPg-NeyCJc0qOrA-1 Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2c032e30083so147358071fa.2 for ; Tue, 26 Sep 2023 23:30:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695796218; x=1696401018; 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=HFnib67QhgCaaUoKsZWlp2Vwzgh+j7FgnEi3HnglTX4=; b=J7Oi7UgAtZkN3krkpfaWfvDGYo5uAh1ad1EcFgrpPYtMZsGVKED3z8PnsFrFfJYHA8 DuYNL0btusW5Jm4nQ8cQ72JdQYam2sWyuppiKMaRBxKIcvhToztJKP4zm06QxkI7o4EZ kiyT+pR5T86o6/vpbxOi4hLBU5aDkcpCfxZmYKdJweXTnGjqzn/YOwYcy9uEJMzPkVzM /i6UKMUly2Z8+7NANv3dUFPP+HC7xCtwa69UbjB5t9jYGus9q5qpkhJuJLQLzpgbhaej YRY4VJyamz0ekL6ydGzuB/imKVnIE+Kx+bnVq3XUBq3bu9paz0iffAdITH5UV+F/LjlW Mjtw== X-Gm-Message-State: AOJu0YwxpVwAUpWDTqlc7wil3fKMwr9WQNzKPq+OGc+jqX4C7gEtWZnv fMlH7++MOsSWDMUlg68H57yKzWyAlPTrBXoshenU6x9Cc9D2RUmqiWeAtGAr3+cj0Lbty8vKDaL xcGqGwRODIRg6rzeMCaA= X-Received: by 2002:a2e:88cf:0:b0:2bd:14cd:3d9 with SMTP id a15-20020a2e88cf000000b002bd14cd03d9mr1108136ljk.25.1695796217791; Tue, 26 Sep 2023 23:30:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFKi9FyIAj9n8eBdAK3qGasXQ882X0+/ux7lCRBDP387PdTMVxQlXo2pCncNuUg1sC2oWJLacFNUa8SnG/DQdA= X-Received: by 2002:a2e:88cf:0:b0:2bd:14cd:3d9 with SMTP id a15-20020a2e88cf000000b002bd14cd03d9mr1108116ljk.25.1695796217459; Tue, 26 Sep 2023 23:30:17 -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 08:30:05 +0200 Message-ID: Subject: Re: [PATCH 0/2] add checks for tests not in a suite To: Aaron Conole , Bruce Richardson 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 Tue, Sep 26, 2023 at 5:01=E2=80=AFPM Aaron Conole w= rote: > 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 defined u= sing > >> > > REGISTER_TEST_COMMAND > >> > > * In checkpatches, add a check to prevent the addition of new test= s > >> > > 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 solution= s. I > >> did it that way to avoid having yet another script to scan the files -= I > >> figured it was faster (in terms of runtime, not dev time) to do the > > > > I had figured it was "faster dev time" that won :-). > > I am fine with it, I don't expect more complications in this area in th= e 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 checkpa= tch > >> 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 testsuites, > > 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 classifying > 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. --=20 David Marchand