Hi, Thanks for looping back to this.
On Sat, 2022-04-16 at 13:28 +0200, Marius Kriegerowski wrote: > After some time I found the time and took a deeper dive into patchtest. It’s > not > in the prettiest state and has a couple of design choices which make me wonder > if it wouldn’t be better to go for a complete rewrite. e.g. I would rather opt > for pytest as it’s much more lightweight and easier to work with and more > modern > than unittest. But let me dive deeper first. I've wondered if we do need to re-invent it but for different reasons. It currently integrates with patchwork but for example it may be better to have it work off lore and it may be better using some of the more modern patchwork tooling. One example I was pointed at was: https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/git-patchwork-bot.py With regard to unitest vs pytest, please do use unittest. The reason I say that is the rest of the project's testing runs within unitest so I'd prefer to have one standard rather than two. You can see it here: https://git.yoctoproject.org/poky/tree/meta/lib/oeqa (there are selftests, runtime tests, sdk tests, eSDK tests, performance tests and so on). > Patchtest is hardly documented which makes it hard to follow. Or is there any > more documentation out there outside of the repo? I would start writing > documentation but want to make sure that I don’t repeat what others have done > before. Sadly there isn't :(. > There are selftests but they don’t work: > https://github.com/HerrMuellerluedenscheid/patchtest/tree/overhaul/selftest > Can these be removed? That directory appears to contain a load of training/test data. I don't care so much about the actual test mechanism but surely the old training data is a good place to start to test any new instance that is brought up? > I’m assuming that in the CI context the `--json` flag is used to produce json > output which is then somehow parsed and returned to the user, right? I would > like to work with the DevOps integration team to make sure I’m not deviating > too > far from what the integration team expects. It looks like you're working on "patchtest-oe" from https://git.yoctoproject.org/patchtest-oe/ . There is a second piece to the puzzle, a harder piece unfortunately which is https://git.yoctoproject.org/patchtest/ . The latter is the piece which is effectively the glue logic between patchwork and patchtest-oe. I'm not sure what you're aiming for as an end result. To recap, what used to happen is that patchwork+patchtest would see new patches on the mailing list, pick them up, run the tests in patchtest-oe against them, then reply on the mailing list if there were any issues detected. We would like to get back to this. Unfortunately we were running a forked/hacked up version of patchwork which we couldn't update. We ended up switching to a vanilla upstream version of it so it is now here: https://patchwork.yoctoproject.org/ however that broke patchtest. What we'd like to do is: a) Get patchtest back up and running against new patches on the mailing list in some form. As I hinted at above, this may mean using a different approach to what patchtest used to do. b) Move the actual tests in patchtest-oe to meta/lib/oeqa/ in OE-Core as a new set of tests. c) Add a script in OE-Core so that you can run the tests in patchtest-oe (now in OE-Core) against a patch before you send it to the mailing list d) Anyone else could then reuse these tests as part of CI, e.g. on github/gitlab/whatever > I managed to make it run on my machine but even if one test fails, it returns > OK in the end. Was that implemented on purpose to make this work in the CI > context? I suspect patchtest would have code to read and handle that. > > So, just for me to know if I’m on the right track, does this look like a > reasonable output indicating that it works fine? > > ❯ patchtest ../poky/0001-scriptutils-fix-style-to-be-more-PEP8-compliant.patch > ../poky ../patchtest-oe/tests > Testing patch ../poky/0001-scriptutils-fix-style-to-be-more-PEP8- > compliant.patch > run pre-merge tests > BUILDDIR is not defined. Cannot load bitbake > SKIP setUpClass (test_metadata_lic_files_chksum.LicFilesChkSum) > BUILDDIR is not defined. Cannot load bitbake > SKIP setUpClass (test_metadata_src_uri.SrcUri) > SKIP test_python_pylint.PyLint.pretest_pylint > ---------------------------------------------------------------------- > Ran 1 test in 0.548s > > OK > run post-merge tests > PASS test_mbox_author.Author.test_author_valid > PASS test_mbox_author.Author.test_non_auh_upgrade > PASS test_mbox_bugzilla.Bugzilla.test_bugzilla_entry_format > SKIP test_mbox_description.CommitMessage.test_commit_message_presence > PASS test_mbox_format.MboxFormat.test_mbox_format > PASS test_mbox_mailinglist.MailingList.test_target_mailing_list > FAIL test_mbox_merge.Merge.test_series_merge_on_head > PASS test_mbox_shortlog.Shortlog.test_shortlog_format > PASS test_mbox_shortlog.Shortlog.test_shortlog_length > FAIL test_mbox_signed_off_by.SignedOffBy.test_signed_off_by_presence > BUILDDIR is not defined. Cannot load bitbake > SKIP setUpClass (test_metadata_lic_files_chksum.LicFilesChkSum) > BUILDDIR is not defined. Cannot load bitbake > SKIP setUpClass (test_metadata_license.License) > SKIP test_metadata_max_length.MaxLength.test_max_line_length > BUILDDIR is not defined. Cannot load bitbake > SKIP setUpClass (test_metadata_src_uri.SrcUri) > BUILDDIR is not defined. Cannot load bitbake > SKIP setUpClass (test_metadata_summary.Summary) > SKIP test_patch_cve.CVE.test_cve_tag_format > SKIP test_patch_signed_off_by.PatchSignedOffBy.test_signed_off_by_presence > SKIP > test_patch_upstream_status.PatchUpstreamStatus.test_upstream_status_presence_f > ormat > SKIP test_python_pylint.PyLint.test_pylint > ---------------------------------------------------------------------- > Ran 15 tests in 0.260s > > OK I believe that patchtest had some minimal bitbake environment which would have allowed some of the skipped tests to run. I'm not an expert on this and I don't remember how the handoff happened with it though. Hope this offers at least some guideance on where things were and where we need to be, at least in my view! :) Cheers, Richard
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#1523): https://lists.openembedded.org/g/openembedded-architecture/message/1523 Mute This Topic: https://lists.openembedded.org/mt/88763351/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
