[1/1] tools: check for pending test status when parsing emails

Message ID 20240517192222.20555-2-probb@iol.unh.edu (mailing list archive)
State Superseded
Headers
Series pending results parsing for DPDK patchwork |

Commit Message

Patrick Robb May 17, 2024, 7:22 p.m. UTC
  Signed-off-by: Patrick Robb <probb@iol.unh.edu>
---
 tools/update-pw.sh | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Patrick Robb May 17, 2024, 7:24 p.m. UTC | #1
+Aaron Conole
+Ali Alnubani

Sorry, I mistakenly didn't add you guys to CC
  
Ali Alnubani May 20, 2024, 6:08 a.m. UTC | #2
> -----Original Message-----
> From: Patrick Robb <probb@iol.unh.edu>
> Sent: Friday, May 17, 2024 10:22 PM
> To: ci@dpdk.org
> Cc: probb@iol.unh.edu; ahassick@iol.unh.edu
> Subject: [PATCH 1/1] tools: check for pending test status when parsing emails
> 
> Signed-off-by: Patrick Robb <probb@iol.unh.edu>
> ---
>  tools/update-pw.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/update-pw.sh b/tools/update-pw.sh
> index 07067dd..b0f0baa 100755
> --- a/tools/update-pw.sh
> +++ b/tools/update-pw.sh
> @@ -49,6 +49,7 @@ case $status in
>  	'SUCCESS') pwstatus='success' ;;
>  	'WARNING') pwstatus='warning' ;;
>  	'FAILURE') pwstatus='fail' ;;
> +	'PENDING') pwstatus='pending' ;;
>  esac
>  printf 'id = %s\nlabel = %s\nstatus = %s/%s %s\nurl = %s\n' \
>  	"$pwid" "$label" "$status" "$pwstatus" "$desc" "$url"
> --
> 2.40.0

Acked-by: Ali Alnubani <alialnu@nvidia.com>
  
Thomas Monjalon May 20, 2024, 7:03 p.m. UTC | #3
17/05/2024 21:22, Patrick Robb:
> Signed-off-by: Patrick Robb <probb@iol.unh.edu>

Please could you explain what it is doing?
Having a workflow understanding would be nice.

> --- a/tools/update-pw.sh
> +++ b/tools/update-pw.sh
> @@ -49,6 +49,7 @@ case $status in
>  	'SUCCESS') pwstatus='success' ;;
>  	'WARNING') pwstatus='warning' ;;
>  	'FAILURE') pwstatus='fail' ;;
> +	'PENDING') pwstatus='pending' ;;
  
Patrick Robb May 20, 2024, 9:36 p.m. UTC | #4
On Mon, May 20, 2024 at 3:03 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 17/05/2024 21:22, Patrick Robb:
> > Signed-off-by: Patrick Robb <probb@iol.unh.edu>
>
> Please could you explain what it is doing?
> Having a workflow understanding would be nice.

Yes good idea.

For context, pending is already a supported check state in patchwork
server: https://patchwork.readthedocs.io/en/latest/usage/overview/#checks

1. DPDK patch is submitted. Patch is acquired by UNH Lab.
2. UNH Lab triggers some testrun pipelines in our CI system (jenkins).
The first action the pipeline takes is to create in our database a
test result record for each testrun, setting the status to PENDING. It
is important to note that one patchwork context, Like
"iol-compile-amd64-testing," may consist of many individual testruns,
each for different distros, hardware, environment etc.
3. When each testrun completes, it will send a report to Patchwork
with the new result (pass or fail). When it does this it will update
the context's results table, changing the environment's result from
pending to pass/fail. So, when the first report comes in for, say,
context "iol-compile-amd64-testing," you would see 1 pass/fail, 12
pending, or similar. Then, as subsequent testruns complete, and report
their results, the updated table comes with the new report. The
overall context result (the _Testing {PASS/FAIL/PENDING}_ at the top
of the test report email) is determined in the manner you might
expect, i.e. if there is at least one testrun fail result, overall
context is fail, else if there is at least one pending result, overall
context is pending, else if all results are passing, overall result is
passing. As an example, when testing is nearly complete, the top of
the report email may look like this:

_Testing PENDING_

Branch: tags/v22.11

a409653a123bf105970a25c594711a3cdc44d139 --> testing pass

Test environment and result as below:

+------------------------------------+-----------------------------------------------------+
|            Environment             |       dpdk_meson_compile      |
+====================================+====================+
| Ubuntu 20.04 ARM SVE                          | PASS               |
+------------------------------------+--------------------+
| Debian 12 with MUSDK                           | PENDING        |
+------------------------------------+--------------------+
| Fedora 37 (ARM)                                     | PASS               |
+------------------------------------+--------------------+
| Ubuntu 20.04 (ARM)                                | PASS               |
+------------------------------------+--------------------+
| Fedora 38 (ARM)                                     | PASS               |
+------------------------------------+--------------------+
| Fedora 39 (ARM)                                     | PENDING        |
+------------------------------------+--------------------+
| Debian 12 (arm)                                       | PASS               |
+------------------------------------+--------------------+
| CentOS Stream 9 (ARM)                         | PASS               |
+------------------------------------+--------------------+
| Debian 11 (Buster) (ARM)                        | PASS               |
+------------------------------------+--------------------+
| Ubuntu 20.04 ARM GCC Cross Compile | PASS               |
+------------------------------------+--------------------+


4. Eventually, all testruns are complete for a patchwork context, and
the table switches from pending to pass or fail.

This does not slow the delivery of results, nor does it increase the
number of test report emails sent. We still send only 1 email per
testrun.

This way it is plainly visible to the user when all testing is
complete, and it also flags for the submitter and for CI people if
some infra failure prevents a testrun from completing, or from a
result being properly emailed, etc. The idea is to provide more
complete status updates and check against infra fails better, but
without any adverse effects in user experience or load on the email
server.

>
> > --- a/tools/update-pw.sh
> > +++ b/tools/update-pw.sh
> > @@ -49,6 +49,7 @@ case $status in
> >       'SUCCESS') pwstatus='success' ;;
> >       'WARNING') pwstatus='warning' ;;
> >       'FAILURE') pwstatus='fail' ;;
> > +     'PENDING') pwstatus='pending' ;;
>
>
>
  
Thomas Monjalon May 21, 2024, 4:08 p.m. UTC | #5
20/05/2024 23:36, Patrick Robb:
> 2. UNH Lab triggers some testrun pipelines in our CI system (jenkins).
> The first action the pipeline takes is to create in our database a
> test result record for each testrun, setting the status to PENDING. It
> is important to note that one patchwork context, Like
> "iol-compile-amd64-testing," may consist of many individual testruns,
> each for different distros, hardware, environment etc.
> 3. When each testrun completes, it will send a report to Patchwork
> with the new result (pass or fail). When it does this it will update
> the context's results table, changing the environment's result from
> pending to pass/fail. So, when the first report comes in for, say,
> context "iol-compile-amd64-testing," you would see 1 pass/fail, 12
> pending, or similar. Then, as subsequent testruns complete, and report
> their results, the updated table comes with the new report. The
> overall context result (the _Testing {PASS/FAIL/PENDING}_ at the top
> of the test report email) is determined in the manner you might
> expect, i.e. if there is at least one testrun fail result, overall
> context is fail, else if there is at least one pending result, overall
> context is pending, else if all results are passing, overall result is
> passing. As an example, when testing is nearly complete, the top of
> the report email may look like this:
> 
> _Testing PENDING_
> 
> Branch: tags/v22.11
> 
> a409653a123bf105970a25c594711a3cdc44d139 --> testing pass
> 
> Test environment and result as below:
> 
> +------------------------------------+-----------------------------------------------------+
> |            Environment             |       dpdk_meson_compile      |
> +====================================+====================+
> | Ubuntu 20.04 ARM SVE                          | PASS               |
> +------------------------------------+--------------------+
> | Debian 12 with MUSDK                           | PENDING        |
> +------------------------------------+--------------------+
> | Fedora 37 (ARM)                                     | PASS               |
> +------------------------------------+--------------------+
> | Ubuntu 20.04 (ARM)                                | PASS               |
> +------------------------------------+--------------------+
> | Fedora 38 (ARM)                                     | PASS               |
> +------------------------------------+--------------------+
> | Fedora 39 (ARM)                                     | PENDING        |
> +------------------------------------+--------------------+
> | Debian 12 (arm)                                       | PASS               |
> +------------------------------------+--------------------+
> | CentOS Stream 9 (ARM)                         | PASS               |
> +------------------------------------+--------------------+
> | Debian 11 (Buster) (ARM)                        | PASS               |
> +------------------------------------+--------------------+
> | Ubuntu 20.04 ARM GCC Cross Compile | PASS               |
> +------------------------------------+--------------------+

It is quite strange to receive a new email each time a line of the table is updated.

> 4. Eventually, all testruns are complete for a patchwork context, and
> the table switches from pending to pass or fail.
> 
> This does not slow the delivery of results, nor does it increase the
> number of test report emails sent. We still send only 1 email per
> testrun.

I had not realised that so many emails are sent.
I thought it was 1 patchwork context == 1 email.

> This way it is plainly visible to the user when all testing is
> complete, and it also flags for the submitter and for CI people if
> some infra failure prevents a testrun from completing, or from a
> result being properly emailed, etc. The idea is to provide more
> complete status updates and check against infra fails better, but
> without any adverse effects in user experience or load on the email
> server.

I understand it gives a new information: test is pending.
  
Aaron Conole May 21, 2024, 5:23 p.m. UTC | #6
Patrick Robb <probb@iol.unh.edu> writes:

> On Mon, May 20, 2024 at 3:03 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 17/05/2024 21:22, Patrick Robb:
>> > Signed-off-by: Patrick Robb <probb@iol.unh.edu>
>>
>> Please could you explain what it is doing?
>> Having a workflow understanding would be nice.

+1.  It would usually be used in the commit message to show the
rationale for the change.  Here's a suggestion for an even shorter
distillation from your message below:

 Today, the community CI infrastructure only uses post-result reporting,
 such as "SUCCESS", "FAILED", and "WARNING".  These results get reported
 only after a test finishes.  This creates some confusion about whether a
 test might have been started for the series in question.  It isn't easy
 to tell at-a-glance which tests are currently running for a given patch
 or series.

 This patch aims to introduce support for a "PENDING" state in the CI
 infrastructure.  This allows labs to indicate which tests have started
 and are awaiting results.  That means test writers should now send a
 "PENDING" status for tests as they start, and then update with a
 post-test result after.  With this change, understanding which tests ran
 at-a-glance is something we can achieve.

 This change should have no affect on the actual tests being run.

Maybe a v2 with something like this as the commit message?  WDYT?

> Yes good idea.
>
> For context, pending is already a supported check state in patchwork
> server: https://patchwork.readthedocs.io/en/latest/usage/overview/#checks
>
> 1. DPDK patch is submitted. Patch is acquired by UNH Lab.
> 2. UNH Lab triggers some testrun pipelines in our CI system (jenkins).
> The first action the pipeline takes is to create in our database a
> test result record for each testrun, setting the status to PENDING. It
> is important to note that one patchwork context, Like
> "iol-compile-amd64-testing," may consist of many individual testruns,
> each for different distros, hardware, environment etc.
> 3. When each testrun completes, it will send a report to Patchwork
> with the new result (pass or fail). When it does this it will update
> the context's results table, changing the environment's result from
> pending to pass/fail. So, when the first report comes in for, say,
> context "iol-compile-amd64-testing," you would see 1 pass/fail, 12
> pending, or similar. Then, as subsequent testruns complete, and report
> their results, the updated table comes with the new report. The
> overall context result (the _Testing {PASS/FAIL/PENDING}_ at the top
> of the test report email) is determined in the manner you might
> expect, i.e. if there is at least one testrun fail result, overall
> context is fail, else if there is at least one pending result, overall
> context is pending, else if all results are passing, overall result is
> passing. As an example, when testing is nearly complete, the top of
> the report email may look like this:
>
> _Testing PENDING_
>
> Branch: tags/v22.11
>
> a409653a123bf105970a25c594711a3cdc44d139 --> testing pass
>
> Test environment and result as below:
>
> +------------------------------------+-----------------------------------------------------+
> |            Environment             |       dpdk_meson_compile      |
> +====================================+====================+
> | Ubuntu 20.04 ARM SVE                          | PASS               |
> +------------------------------------+--------------------+
> | Debian 12 with MUSDK                           | PENDING        |
> +------------------------------------+--------------------+
> | Fedora 37 (ARM)                                     | PASS               |
> +------------------------------------+--------------------+
> | Ubuntu 20.04 (ARM)                                | PASS               |
> +------------------------------------+--------------------+
> | Fedora 38 (ARM)                                     | PASS               |
> +------------------------------------+--------------------+
> | Fedora 39 (ARM)                                     | PENDING        |
> +------------------------------------+--------------------+
> | Debian 12 (arm)                                       | PASS               |
> +------------------------------------+--------------------+
> | CentOS Stream 9 (ARM)                         | PASS               |
> +------------------------------------+--------------------+
> | Debian 11 (Buster) (ARM)                        | PASS               |
> +------------------------------------+--------------------+
> | Ubuntu 20.04 ARM GCC Cross Compile | PASS               |
> +------------------------------------+--------------------+
>
>
> 4. Eventually, all testruns are complete for a patchwork context, and
> the table switches from pending to pass or fail.
>
> This does not slow the delivery of results, nor does it increase the
> number of test report emails sent. We still send only 1 email per
> testrun.
>
> This way it is plainly visible to the user when all testing is
> complete, and it also flags for the submitter and for CI people if
> some infra failure prevents a testrun from completing, or from a
> result being properly emailed, etc. The idea is to provide more
> complete status updates and check against infra fails better, but
> without any adverse effects in user experience or load on the email
> server.
>
>>
>> > --- a/tools/update-pw.sh
>> > +++ b/tools/update-pw.sh
>> > @@ -49,6 +49,7 @@ case $status in
>> >       'SUCCESS') pwstatus='success' ;;
>> >       'WARNING') pwstatus='warning' ;;
>> >       'FAILURE') pwstatus='fail' ;;
>> > +     'PENDING') pwstatus='pending' ;;
>>
>>
>>
  
Patrick Robb May 23, 2024, 9:47 p.m. UTC | #7
On Tue, May 21, 2024 at 12:08 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 20/05/2024 23:36, Patrick Robb:
> > 2. UNH Lab triggers some testrun pipelines in our CI system (jenkins).
> > The first action the pipeline takes is to create in our database a
> > test result record for each testrun, setting the status to PENDING. It
> > is important to note that one patchwork context, Like
> > "iol-compile-amd64-testing," may consist of many individual testruns,
> > each for different distros, hardware, environment etc.
> > 3. When each testrun completes, it will send a report to Patchwork
> > with the new result (pass or fail). When it does this it will update
> > the context's results table, changing the environment's result from
> > pending to pass/fail. So, when the first report comes in for, say,
> > context "iol-compile-amd64-testing," you would see 1 pass/fail, 12
> > pending, or similar. Then, as subsequent testruns complete, and report
> > their results, the updated table comes with the new report. The
> > overall context result (the _Testing {PASS/FAIL/PENDING}_ at the top
> > of the test report email) is determined in the manner you might
> > expect, i.e. if there is at least one testrun fail result, overall
> > context is fail, else if there is at least one pending result, overall
> > context is pending, else if all results are passing, overall result is
> > passing. As an example, when testing is nearly complete, the top of
> > the report email may look like this:
> >
> > _Testing PENDING_
> >
> > Branch: tags/v22.11
> >
> > a409653a123bf105970a25c594711a3cdc44d139 --> testing pass
> >
> > Test environment and result as below:
> >
> > +------------------------------------+-----------------------------------------------------+
> > |            Environment             |       dpdk_meson_compile      |
> > +====================================+====================+
> > | Ubuntu 20.04 ARM SVE                          | PASS               |
> > +------------------------------------+--------------------+
> > | Debian 12 with MUSDK                           | PENDING        |
> > +------------------------------------+--------------------+
> > | Fedora 37 (ARM)                                     | PASS               |
> > +------------------------------------+--------------------+
> > | Ubuntu 20.04 (ARM)                                | PASS               |
> > +------------------------------------+--------------------+
> > | Fedora 38 (ARM)                                     | PASS               |
> > +------------------------------------+--------------------+
> > | Fedora 39 (ARM)                                     | PENDING        |
> > +------------------------------------+--------------------+
> > | Debian 12 (arm)                                       | PASS               |
> > +------------------------------------+--------------------+
> > | CentOS Stream 9 (ARM)                         | PASS               |
> > +------------------------------------+--------------------+
> > | Debian 11 (Buster) (ARM)                        | PASS               |
> > +------------------------------------+--------------------+
> > | Ubuntu 20.04 ARM GCC Cross Compile | PASS               |
> > +------------------------------------+--------------------+
>
> It is quite strange to receive a new email each time a line of the table is updated.
>
> > 4. Eventually, all testruns are complete for a patchwork context, and
> > the table switches from pending to pass or fail.
> >
> > This does not slow the delivery of results, nor does it increase the
> > number of test report emails sent. We still send only 1 email per
> > testrun.
>
> I had not realised that so many emails are sent.
> I thought it was 1 patchwork context == 1 email.

This is how it worked until last year, but our test results delivery
was slow in some cases. So, we implemented "tail reporting" i.e. the
ability for an environment to report its own test run when it finishes
testing, as opposed to relying on a testrun aggregator which runs
later. If a test run fails, we want to share that information as soon
as we can, not block it on other testing.
  

Patch

diff --git a/tools/update-pw.sh b/tools/update-pw.sh
index 07067dd..b0f0baa 100755
--- a/tools/update-pw.sh
+++ b/tools/update-pw.sh
@@ -49,6 +49,7 @@  case $status in
 	'SUCCESS') pwstatus='success' ;;
 	'WARNING') pwstatus='warning' ;;
 	'FAILURE') pwstatus='fail' ;;
+	'PENDING') pwstatus='pending' ;;
 esac
 printf 'id = %s\nlabel = %s\nstatus = %s/%s %s\nurl = %s\n' \
 	"$pwid" "$label" "$status" "$pwstatus" "$desc" "$url"