This patch adds a new flag to lit --xfail-requires-pr which causes lit tests to fail if any XFAIL lines do not also reference a PR. To support this functionality this patch changes how XFAIL lines are parsed to allow one entry in the comma separated list to be a PR (starting with the string "PR").
This is great idea. Some comments:
- I'd make it require a full URL like http://llvm.org/PRxxxx instead of just PRxxxx so it is easy to get the PR (I am a big fan of Cmd+Click on URL on terminals :)
- Instead of making it a commandline option, I'd rather put the setting into lit.cfg/lit.site.cfg so it will be enforced based on which tests we are running.
@MatzeB, that's a great suggestion. I'll include that in my rework. I also need to add a method of supporting external bug trackers for out-of-tree projects.
- Made PR pattern match LLVM.org URL
- Added support for adding extra PR prefixes
- Added tests and fixed the bugs I found because of them
You should use test.config.xFailRequirePR, test.config.bugTrackerRegex after adding some default values to TestingConfig.py. That way these values can be specified in lit.cfg and lit.site.cfg rather than the lit commandline. That will ensure nobody forgets to use those flags and also allows to run test from two testsuites with different bugtrackers in a single lit run.
As a downstream user I would definitely want to put my additional private prefix in a config file. Don't want to have to remember extra command-line options when running tests individually.
I had not intended this patch to be the actual review thread (hence not including llvm-commits). I'm going to adapt to the feedback here and re-post to llvm-commits since the llvm-dev thread was mostly favorable.