Re: Email Based Re-Testing Framework

[Patrick Robb <probb@iol.unh.edu>]

[-- Attachment #1: Type: text/plain, Size: 4720 bytes --]

On Wed, Jun 7, 2023 at 8:53 AM Aaron Conole <aconole@redhat.com> wrote:

> Patrick Robb <probb@iol.unh.edu> writes:
>
> >  Also it can be useful to run daily sub-tree testing by request, if
> possible.
> >
> > That wouldn't be too difficult. I'll make a ticket for this. Although,
> for testing on the daily sub-trees, since that's
> > UNH-IOL specific, that wouldn't necessarily have to be done via an email
> based testing request framework. We
> > could also just add a button to our dashboard which triggers a sub-tree
> ci run. That would help keep narrow
> > the scope of what the email based retesting framework is for. But, both
> email or a dashboard button would
> > both work.
>
> We had discussed this long ago - including agreeing on a format, IIRC.
>
> See the thread starting here:
>   https://mails.dpdk.org/archives/ci/2021-May/001189.html
>
> The idea was to have a line like:
>
> Recheck-request: <test names>
>
I like using this simpler format which is easier to parse. As Thomas
pointed out, specifying labs does not really provide extra information if
we are already going to request by label/context, which is already
specifies the lab.

>
> where <test names> was the tests in the check labels.  In fact, what
> started the discussion was a patch for the pw-ci scripts that
> implemented part of it.
>
> I don't see how to make your proposal as easily parsed.
>
> WDYT?  Can you re-read that thread and come up with comments?

 Will do. And thanks, this thread is very informative.

> It is important to use the 'msgid' field to distinguish recheck
> requests.  Otherwise, we will continuously reparse the same
> recheck request and loop forever.  Additionally, we've discussed using a
> counter to limit the recheck requests to a single 'recheck' per test
> name.
>
We can track message ids to avoid considering a single retest request
twice. Perhaps we can accomplish the same thing by tracking retested
patchseries ids and their total number of requested retests (which could be
1 retest per patchseries).

>  +function check_series_needs_retest() {

+    local pw_instance="$1"
+
+    series_get_active_branches "$pw_instance" | while IFS=\| read -r
series_id project url repo branchname; do
+        local patch_comments_url=$(curl -s "$userpw" "$url" | jq -rc
'.comments')
+        if [ "Xnull" != "X$patch_comments_url" ]; then
+            local comments_json=$(curl -s "$userpw" "$patch_comments_url")
+            local seq_end=$(echo "$comments_json" | jq -rc 'length')
+            if [ "$seq_end" -a $seq_end -gt 0 ]; then
+                seq_end=$((seq_end-1))
+                for comment_id in $(seq 0 $seq_end); do
+                    local recheck_requested=$(echo "$comments_json" | jq
-rc ".[$comment_id].content" | grep "^Recheck-request: ")
+                    if [ "X$recheck_requested" != "X" ]; then
+                        local msgid=$(echo "$comments_json" | jq -rc
".[$comment_id].msgid")
+                        run_recheck "$pw_instance" "$series_id" "$project"
"$url" "$repo" "$branchname" "$recheck_requested" "$msgid"
+                    fi
+                done
+            fi
+        fi
+    done
+}
This is already a superior approach to what I had in mind for acquiring
comments. Unless you're opposed, I think at the communit lab we can
experiment based on this starting point to verify the process is sound, but
I don't see any problems here.

> I think that if we're able to specify multiple contexts, then there's not
> really any reason to run multiple rechecks per patchset.
>
Agreed.

> There was also an ask on filtering requesters (only maintainers and

patch authors can ask for a recheck).

If we can use the maintainers file as a single source of truth that is
convenient and stable as the list of maintainers changes. But, also I think
retesting request permission should be extended to the submitter too. They
may want to initiate a re-run without engaging a maintainer. It's not
likely to cause a big increase in test load for us or other labs, so
there's no harm there.

> No, an explicit list is actually better.

When a new check is added, for someone looking at the mails (maybe 2/3

weeks later), and reading just "all", he would have to know what

checks were available at the time.

Context/Labels rarely change, so I don't think this concern is too serious.
But, if people dont mind comma separating an entire list of contexts,
that's fine.

Thanks,
Patrick

-- 

Patrick Robb

Technical Service Manager

UNH InterOperability Laboratory

21 Madbury Rd, Suite 100, Durham, NH 03824

www.iol.unh.edu

[-- Attachment #2: Type: text/html, Size: 9041 bytes --]