Bug 8648

Created attachment 2856 [details]
Patch workflow between submitter/maintainer and patchwork

This is a proposed workflow for merging and testing patches coming from the mailing list. The introduction of patch's tags are important to keep track of the patch status. This workflow takes into account the current bug and 8649 [1]

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=8649

Leo, I would highly recommend using containers to do the tests against the patch. I will shortly be sending some instructions to other GDC guys with some container testing I had setup, you may be able to leverage that somehow.

If you have questions, let me know.

(In reply to comment #2)
> Leo, I would highly recommend using containers to do the tests against the
> patch. I will shortly be sending some instructions to other GDC guys with
> some container testing I had setup, you may be able to leverage that somehow.
> 

Thanks Randy. Looking forward to see the instructions.

> If you have questions, let me know.

I have found this useful tool call git-pw [1]. I have not fully used the tool, but apparently for testing new patchwork patches into a repository, it would be a matter of using this tool with apply and apply-patch parameters. It is interesting to notice that this tool is part of patchwork repository [2] maintained by Damien Lespiau;

[1]
http://patchwork-freedesktop.readthedocs.org/en/latest/manual.html#git-pw
[2] https://github.com/dlespiau/patchwork

git-pw could definitely be useful; an interesting challenge we have though is that we often receive (and encourage submitters to send) pull requests which come with a git branch, and it's fairly common practice for that branch to be updated without sending a new patch when making a minor change (such as fixing a typo or commit message); AIUI Ross and Richard tend to grab the branch rather than the individual patches because it's easier, so if they differ from the patches posted to the list that could be an issue.

It may be that the simplest way of handling this is to ban the practice of updating the git branch without reposting a new version of at least the affected patch. We have received complaints in the past from at least one community member about branches not matching with the posted patches. However, even if we did this we'd have to have a check somewhere to verify that the patches do in fact correspond to the branch.

(This is perhaps something to consider as a separate work task, but we do need to keep it in mind.)

Paul, Leonardo, as you know, the nature of branch:HEADs is volatile, when you send a branch for review and later you update such branch then the review material gets outdated automatically.

I've seen in the past that working around such behavior of the branches could be done via specifying branch:sha or using git tags. Both of them with pros/cons and involves changes in the process that are not easily taken by the community as they usually mean extra work.

It could be proposed that with the merge-request that a developer sends, it includes the sha or it gets tagged (bug number perhaps?) so the reference does not move if the branch gets updated.

I missed M1. This feature will be tied to the git-pw tool, tool present on the damien's PW repo. Some sub-commands are missing on this tool, thus focusing on it, specially [1] and [2].


[1] https://github.com/dlespiau/patchwork/issues/115
[2] https://github.com/dlespiau/patchwork/issues/113

Some scenarios using this tool:

# Test if 'last' (the last series sent to the ML) series apply cleanly
> git pw list --since XXX | get_id | head -1 | xargs git pw apply

# Test all series since a specific date
> git pw list --since XXX | get_id | xargs git pw apply

# Execute some static tests on mboxes since a specific date
> git pw list --since XXX | get_id | xargs git pw mbox | checkpatch.pl

M2 missed. 

Most time spent on the new tool to test patches called 'patchtest' [1]. The tool is ready to be used, and currently being tested. patchtest is just a test framework and it is not intended to include test cases/units. So all dependencies bugs should be done in a separate repo and this effort is being done under [2] repository at the moment.

[1] https://github.com/lsandoval/patchtest
[2] https://github.com/aphran/pt-suites

This enhancement was not completed on the enhancement period. Effort done by Daniel but still not fully complete. Work will be taken again on the 2.2 period.

By mistake, I moved to it to 2.2 but it should be 2.1 M4, as set now. The last comment I was referring to [1]

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=8647

Most of the desired features are now supported on [1] but testing on [2] is still pending. At least moving to a more realistic status.

[1] https://github.com/lsandoval/patchtest
[2] http://patchwork.openembedded.org/

Michael, please assign this bug back to me once the PW instance is ready. Patchtest is practically ready to query/post the new instance.

Waiting on Patchwork upgrade.

I wanted to clarify that as far as I understand this bug is blocked by bug 9130 which I am primarily responsible for. Jose Lamego and Paul Eggleton are also working on that bug with me. Once we are finished upgrading Patchwork Leonardo Sandoval Gonzalez will be able to continue with this bug.

Now that new instance is up, I need to test fetching series and posting results to it, so taking the bug again.

patchtest system has been implemented but not fully tested on 'live' patchwork series events. The reason to move it to M2 is to have this window to test incoming patches using patchest-oe as a test suite (and patchtest as an engine). Test results wont be posted to patchwork; instead, the behavior of the system will be monitored and if necessary, changes will be done. This way, we can do changes into the system and developer does not receive any email on behalf of patchtest. Repository is still not public.

On the testing phase with new series and new checks located on the suite. Repository is still not public.

+ Framework is complete (still not public). 
+ Accepts a single patch (fetched from patchwork or created with git-format-patch) and expects a test suite to be executed.
+ The related test suite to be used is being tracked on https://bugzilla.yoctoproject.org/show_bug.cgi?id=4078
+ Current command line arguments:

> patchtest --help
usage: patchtest [-h] [-C REPODIR] [--test-name TESTNAME]
                 [--keep-branch | --no-apply] [--test-dir TESTDIR]
                 [--branch BRANCH] [--commit COMMIT] [--debug] [--failures]
                 [--raw]
                 PATCH

positional arguments:
  PATCH                 The patch to be tested by patchtest

optional arguments:
  -h, --help            show this help message and exit
  -C REPODIR            Name of the repository where testing is done
  --test-name TESTNAME  Test name to be used if results are posted. In case
                        all items failed merged, then the test name is
                        <--test-name>-merge-failure
  --keep-branch         Keep the working branch after patchtest execution
  --no-apply            Do not apply the patch into the repo
  --test-dir TESTDIR    Directory where tests are located
  --branch BRANCH, -b BRANCH
                        Branch name used by patchtest to branch from. By
                        default, it uses the current one.
  --commit COMMIT, -c COMMIT
                        Commit ID used by patchtest to branch from. By
                        default, it uses HEAD.
  --debug, -d           Enable debug output
  --failures, -f        Print just failures
  --raw, -r             Print errors coming from the test suite

I'm sorry, but as we discussed before this cannot be marked resolved until we're actually testing incoming patches, and we aren't yet doing that.

(In reply to comment #19)
> I'm sorry, but as we discussed before this cannot be marked resolved until
> we're actually testing incoming patches, and we aren't yet doing that.

ok. patchtest is working now but you have not seen the emails yet produced by patchtest. I will forward these to you until M2 release. For the moment, I am  using an internal patchwork server and posting goes to a particular series (I am the author) no matter what the series is being tested, so I am getting all emails for the moment.

Being reviewed currently

+ Patchtest and Patchtest-oe code was reviewed by Paul Eggleton. All observations covered except https://bugzilla.yoctoproject.org/show_bug.cgi?id=10049.
 
+ Live testing is still pending but is now tracked on a separated bug:  https://bugzilla.yoctoproject.org/show_bug.cgi?id=10057

+ A full run covering series from an internal patchwork instance was done and emails (sent by patchwork on behalf of patchtest) sent to Paul E. and myself. Summary looks good but 10049 is still pending. Several  false positives checks found and these are currently being corrected. All this work is related to bug: https://bugzilla.yoctoproject.org/show_bug.cgi?id=9244.

+ Future issues regarding patchtest (testing framework) or patchtest-oe (patch test suite) should be reported in separately.