[DRAFT / RFC] Add PR checks for build/test/install #11019
Conversation
There was a problem hiding this comment.
Thanks @reubeno for taking it forward!
I've made quick observation about introducing hints in the spec-file. I could be wrong. Please let me know if I miss any advantage in running test and ignoring the results over not running the tests at all.
I think this PR can be split into multiple chunks.
- Capture test output into a machine readable JSon format
- Introduction of run_ptests.py scripts and necessary parameters
- Introduction of install/uninstall tests
- Build-check workflow for GitHub
Because, splitting the PR will let us to avoid unintentional inter-dependency.
I'm continuing to review the PR. I'll add more comments the more I understand the changes.
| @@ -38,6 +38,7 @@ cached_file = $(PKGBUILD_DIR)/cached_graph.dot | |||
| preprocessed_file = $(PKGBUILD_DIR)/preprocessed_graph.dot | |||
| built_file = $(PKGBUILD_DIR)/built_graph.dot | |||
| output_csv_file = $(PKGBUILD_DIR)/build_state.csv | |||
| test_results_file = $(PKGBUILD_DIR)/test_results.json | |||
There was a problem hiding this comment.
Need tabs to be converted to spaces. In other places too.
| @@ -704,6 +713,49 @@ func readBuildRequires(specFile, sourceDir, arch string, defines map[string]stri | |||
| return | |||
| } | |||
|
|
|||
| func querySpecForTestFailureExpectation(specFile string) (result bool, err error) { | |||
There was a problem hiding this comment.
IMHO it is little over complication. If the spec writer knows that the PTest is expected to fail, they can simply disable the check section. So, instead of imposing everyone to add the special comment, we can ask them to disable the check.
There was a problem hiding this comment.
Right now we find ourselves with a large number of failing tests. We're going to need to make a bulk change to baseline those known-failing tests. I'm definitely open to other approaches for capturing it if you have alternate recommendations.
And note I intentionally don't want to disable the check section. If a test is failing, IMHO we should still run it but not report the failure as blocking.
100% agreed. I'd still like to get feedback on the approach before doing that, though. |
(Draft change -- requesting feedback/comments)
This is an initial attempt at adding PR checks that detect changed specs, build + test them, and try to install/uninstall the built RPMs. For an example run, see this private run.
Note that the install/uninstall script is an as-is copy from existing ADO pipeline scripts.
The changes to the toolkit golang code needs the most feedback (e.g., the emitting of a tests results JSON). Specifically note the proposal to allow a .spec to use a specialized comment in this line:
to denote a baselined ptest failure.