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 0CBF7A0C43 for ; Wed, 1 Sep 2021 14:50:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E3F5240140; Wed, 1 Sep 2021 14:50:24 +0200 (CEST) Received: from mail-ot1-f49.google.com (mail-ot1-f49.google.com [209.85.210.49]) by mails.dpdk.org (Postfix) with ESMTP id 5995E40140 for ; Wed, 1 Sep 2021 14:50:23 +0200 (CEST) Received: by mail-ot1-f49.google.com with SMTP id o16-20020a9d2210000000b0051b1e56c98fso3273972ota.8 for ; Wed, 01 Sep 2021 05:50:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=31KaGsEzRzRJmnIh6o/nBfyVGne1wqPmTqYNV1k4UI8=; b=WWiXyhvs4323bEf0sxk74s60Ni+6nICa18g7wnGZ5l8XOL6PnAQAEkYwStT3LZaKU6 DzkkBJ96EdkH/iXzbUeMHvs1Fuj27SWh40in8TLvEmh4mlO0RyvB0UgftrH637ZkTxNm ji4mMV3ZOcoIxc1Vs1Utt53Kl6ZLlWlD7gVDk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=31KaGsEzRzRJmnIh6o/nBfyVGne1wqPmTqYNV1k4UI8=; b=eTtQ6HPOAXQpJCOOK/wis0LYfHScpJozEp59NuQbJJUq5aap2bsUM2vD4NRKHfW09+ FWS+da/ilgQXQF9Y//bTVSDgtvYAzaAGTZs9r6BVkXMADRy2X5r3g9BFvHd6QdvpUxOY ZAKRPgXgmc03ORmeiZMtXQzIM4EDmWHbZBaC1P0gt6Glk6Wvg69HEoeJXWOpwlE/3Tv6 Kx6QJ5LJ9TUMYoo3cjP/OIk9WKjN7NYCUWhQht1/hTuTn8DHB5XYk8YDxU+161l9N4dx 2hMXq+rgbHg16X2JO8+v3GHF1tcB6A9DGoICzYVaUuj/TLVJn3tx1lFkwIEuM+ozdOFc pEsA== X-Gm-Message-State: AOAM5330RIVBQHP5F1z8ezw/5zdBvofLcgW3283dtVp+uAYS20latNDi qqOfddZgH5y+Z1JggLEsqhJHVH296hHwgK6YScVtcg== X-Google-Smtp-Source: ABdhPJzsyWs77yuCRxndc+yWFwdTEfWREcdnGGDkBEPs9rq8OwD7laoTUA/rrP/9tbpb5oUs/uwt7ADIYR6shXUTzpw= X-Received: by 2002:a05:6830:2f4:: with SMTP id r20mr14943578ote.117.1630500622652; Wed, 01 Sep 2021 05:50:22 -0700 (PDT) MIME-Version: 1.0 References: <1692874.CEL9UuTRB0@thomas> <3224374.3u8ESuPvME@thomas> In-Reply-To: <3224374.3u8ESuPvME@thomas> From: Owen Hilyard Date: Wed, 1 Sep 2021 08:49:50 -0400 Message-ID: To: Thomas Monjalon Cc: Lincoln Lavoie , ci@dpdk.org Content-Type: multipart/alternative; boundary="0000000000006b6de305caee8295" Subject: Re: [dpdk-ci] Community CI Meeting Minutes X-BeenThere: ci@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK CI discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ci-bounces@dpdk.org Sender: "ci" --0000000000006b6de305caee8295 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello Thomas, I was the one who asked for that feature, and I wanted to explain my reasoning. First, it is not possible to run every test in DTS for every patch with a reasonable amount of hardware. The last time I tried to run everything it took well over 2 days. As such, the DTS working group decided that it would be best to target our testing. I wrote a script that walks the python module paths and will use the files changed in a patch to determine which DTS tests could possibly be affected by the change. However, since many of the tests in DTS require special configuration or particular hardware, the Intel Lab has a limited set of tests that can be run. I asked that these warnings be sent out for a test that could be affected by a change, but which was not run. In part, this is due to a situation that can occur like with a recent patch I made to the rte flow test suite, where I basically rewrote a test suite but it was not tested in CI. All of the smoke tests pass, so it looks fine, but given how DTS is set up, I could have placed code that wouldn't compile in the test suite and it would still have passed CI. Since we don't have a good way to prevent that, these notifications are a way for maintainers to quickly check if there were changes in a test suite that was not tested, and either manually test the patch or pay special attention to it. As for your concern about noise in patchwork, this would be run on DTS patches only, and would not report onto the main DPDK section. There is very little going on in the DTS patchworks right now (many patches have no tests at all) and this provides valuable information for maintainers to help streamline patch review. The only time this would generate more than 1 or 2 warnings would be if a change is made to somewhere very deep in the DTS framework. When that happens, I personally would prefer that it generates a lot of output and makes itself noticable, since framework level changes need to be very carefully reviewed to avoid breaking DPDK CI. Owen Hilyard On Tue, Aug 31, 2021 at 11:47 AM Thomas Monjalon wrote: > Then it should not be reported in patchwork. > Please let's not add more noise in patchwork. > > > 26/08/2021 18:27, Lincoln Lavoie: > > This is specific to patches for DTS, where Intel doesn't have the > > infrastructure to run every possible test suite that is included in DTS= . > > So, the warning is a notice to the submitter and the maintainer(s) the > > patch couldn't be tested. It's not really an issue related to the conte= nt > > of the patch itself (i.e. not about a breaking change or something like > > that). > > > > Cheers, > > Lincoln > > > > On Thu, Aug 26, 2021 at 12:05 PM Thomas Monjalon > > wrote: > > > > > 26/08/2021 15:33, Lincoln Lavoie: > > > > * For DTS CI, should authors be notified of skipped testing (i.e. C= I > > > infrastructure doesn=E2=80=99t support that test suite)? Should this= be > marked as > > > a warning in patchwork? Agreed it should do both of these actions, > notify > > > the author and mark the patch as warning in patchworks. > > > > > > Not sure to understand. > > > If a test is not supported, it is not an issue of the patch, > > > so why would it be reported in patchwork? > > > > --0000000000006b6de305caee8295 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello Thomas,

I was the one who asked f= or that feature, and I wanted to explain my reasoning.=C2=A0

=
First, it is not possible to run every test in DTS for every pat= ch with a reasonable amount of hardware. The last time I tried to run every= thing it took well over 2 days. As such, the DTS working group decided that= it would be best to target our testing. I wrote a script that walks the py= thon module paths and will use the files changed in a patch to determine wh= ich DTS tests could possibly be affected by the change. However, since many= of the tests in DTS require special configuration or particular hardware, = the Intel Lab has a limited set of tests that can be run. I asked that thes= e warnings be sent out for a test that could be affected by a change, but w= hich was not run. In part, this is due to a situation that can occur like w= ith a recent patch I made to the rte flow test suite, where I basically rew= rote a test suite but it was not tested in CI. All of the smoke tests pass,= so it looks fine, but given how DTS is set up, I could have placed code th= at wouldn't compile in the test suite and it would still have passed CI= . Since we don't have a good way to prevent that, these notifications a= re a way for maintainers to quickly check if there were changes in a test s= uite that was not tested, and either manually test the patch or pay special= attention to it.=C2=A0

As for your concern about noise in patchwork= , this would be run on DTS patches only, and would not report onto the main= DPDK section. There is very little going on in the DTS patchworks right no= w (many patches have no tests at all) and this provides valuable informatio= n for maintainers to help streamline patch review. The only time this would= generate more than 1 or 2 warnings would be if a change is made to somewhe= re very deep in the DTS framework. When that happens, I personally would pr= efer that it generates a lot of output and makes itself noticable, since fr= amework level changes need to be very carefully reviewed to avoid breaking = DPDK CI.=C2=A0

Owen Hilyard

On Tue, Aug 31, 2= 021 at 11:47 AM Thomas Monjalon <= thomas@monjalon.net> wrote:
Then it should not be reported in patchwork.
Please let's not add more noise in patchwork.


26/08/2021 18:27, Lincoln Lavoie:
> This is specific to patches for DTS, where Intel doesn't have the<= br> > infrastructure to run every possible test suite that is included in DT= S.
> So, the warning is a notice to the submitter and the maintainer(s) the=
> patch couldn't be tested. It's not really an issue related to = the content
> of the patch itself (i.e. not about a breaking change or something lik= e
> that).
>
> Cheers,
> Lincoln
>
> On Thu, Aug 26, 2021 at 12:05 PM Thomas Monjalon <thomas@monjalon.net>
> wrote:
>
> > 26/08/2021 15:33, Lincoln Lavoie:
> > > * For DTS CI, should authors be notified of skipped testing = (i.e. CI
> > infrastructure doesn=E2=80=99t support that test suite)?=C2=A0 Sh= ould this be marked as
> > a warning in patchwork?=C2=A0 Agreed it should do both of these a= ctions, notify
> > the author and mark the patch as warning in patchworks.
> >
> > Not sure to understand.
> > If a test is not supported, it is not an issue of the patch,
> > so why would it be reported in patchwork?



--0000000000006b6de305caee8295--