[LU-9619] automatically add Test-Parameters: line for simple patches

• Status: Open
• Project: Lustre
• Type: Improvement
• Priority: Minor
• Reporter: Andreas Dilger
• Assignee: Tim Day
• Resolution: Unresolved
• Labels: easy, tests

Description

It would be possible to have the contrib/git-hooks/prepare-commit-message script automatically add a Test-Parameters: trivial tag (with an optional testlist=<test> item) to patches that are only modifying test scripts. As with the Signed-off-by: line, this should be added in a commented-out form so that it is up to the developer to determine if this is appropriate, but having the line present would at least remind the user about this option to reduce testing time.

Comments

[Gerrit Updater]

"Timothy Day <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/50298
Subject: LU-9619 misc: add Test-Parameters to git-hook
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: de150d9f0479eafdae5a2b0d28ca8297fd6afd4c

[Andreas Dilger]

Tim, as a follow-on to your above patch, it would be useful if prepare-commit-msg could add a "real" Test-Parameters: lines (not just examples) based on the output of "git diff $DIFFOPT --stat".

For example, if the patch is only modifying files like "lustre/tests/conf-sanity.sh" then the commit message could add:

Test-Parameters: trivial testlist=conf-sanity

Similarly, though a bit more complex, would be if patches add lines in test scripts with version_code then it makes sense to automatically add lines that run interop testing:

Test-Parameters: testlist=<test_script> serverversion=<older_version>
Test-Parameters: testlist=<test_script> clientversion=<older_version>

This is a bit tricky if it is kernel or distro versions being checked, but it couldn't hurt to highlight this at least (maybe also checkpatch.pl, so that it is posted in Gerrit as a reminder for reviewers).

Also, commit-msg could potentially sanity-check any existing Test-Parameters: lines to confirm that either no trivial keyword is added, or that every test script in lustre/tests/*.sh is explicitly listed with testlist= (excluding "sanity.sh because that is always run regardless of any Test-Parameters: specified). However, I'd prefer the "carrot" approach that automatically adds the Test-Parameters: lines rather than complaining afterward.

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/50298/
Subject: LU-9619 misc: add Test-Parameters to git-hook
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: a7cd87862470899fe6ceb08b50267ea058221a13

[Andreas Dilger]

Oleg's code for determining tests to be run based on the patch content is in https://github.com/verygreen/lustretester/blob/master/gerrit_build-and-test-new.py

[Andreas Dilger]

timday, are you planning to work on this any further? I think automating the generation of Test-Parameters: lines in the prepare-commit-message Git hook would optimize the use of testing resources, and simplify the submission of larger patch series (which in turn would make patch reviews easier to do). I've linked Oleg's script to do something similar for the Gerrit Janitor testing, but doing something similar for Test-Parameters: would also help Autotest.

[Gerrit Updater]

"Timothy Day <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/52420
Subject: LU-9619 contrib: rewrite prepare-commit-msg in python
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 4fd637ebe2f6fed1ac3da8fc5ee044cd61f271ee