cms-bot
cms-bot copied to clipboard
Use another PR in baseline for comparison tests
During the development of conditions, AlCa finds it useful to separate PRs that update the global tags in autoCond into logically separate pull requests. In particular, coupled code+conditions updates should not introduce additional, unrelated condition changes. This helps to cross-check that no changes were introduced into workflows other than those intended. For example:
- https://github.com/cms-sw/cmssw/pull/27644 should only affect 2016+2017 data
- https://github.com/cms-sw/cmssw/pull/27698 should only affect 2018 data and MC
- https://github.com/cms-sw/cmssw/pull/27651 should only affect 2021 MC
- the 10_6_X backport of https://github.com/cms-sw/cmssw/pull/27644 should only affect 2018 and later data and non-2017 MC
The comparison tests currently make comparisons against the most recent IB but global tags are created and approved outside the CMSSW release framework. So, for example, the GTs in https://github.com/cms-sw/cmssw/pull/27644 are final and all future global tags will include these updates. Where there are conflicts between the global tags in open PRs, global tags in the PRs that are intended to be merged later (to maintain consistency with the global tag versioning) are constructed relative to the global tags in autoCond (rather than relative to the global tag queues) purely for the purpose of supporting the comparison tests.
It would be better if we could use the correct global tag, based on the content of the global tag queue, in the PR from the beginning. To support the PR tests in this case, a new syntax would need to be introduced to the bot to use additional PRs in the baseline for the comparison tests. For example:
[@cmsbuild,] please test baseline <#PR[,#PR[...]]>
Then https://github.com/cms-sw/cmssw/pull/27698 would be tested with
please test baseline 27644
and https://github.com/cms-sw/cmssw/pull/27644 would be tested with
please test baseline 27698,27651
where https://github.com/cms-sw/cmssw/pull/27698 would also include the updated global tag content of https://github.com/cms-sw/cmssw/pull/27644.
A new Issue was created by @christopheralanwest .
@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.
cms-bot commands are listed here
Related to #937
It seems like we need a general scheme to generate, store, and track alternative comparison baselines.
@christopheralanwest , we have the results for PRs and I think with little modification we can use that as baseline. So if I understand correctly you will be doing
- cms-sw/cmssw#27651: please test
- Use latest IB and normal run normal tests
- Use IB as baseline
- cms-sw/cmssw#27698: please test baseline #27651
- Pickup IB used for testing #27651 PR and build and test #27698 and #27651
- Use baseline of #27651 which should have been uploaded before issuing this command.
- cms-sw/cmssw#27644: please test baseline #27651,#27698
- Pickup IB used for testing #27651 and build and test #27644, #27698 and #27651
- Use baseline of #27651,#27698 which should have been uploaded already.
Note that in this case one can only start the tests once a baseline of other PR is already available. Also you can not say e.g. for cms-sw/cmssw#27644 please test baseline #27698
as baseline for only #27698 was never generated and uploaded.
@smuzaffar What you propose isn't actually what I intended nor would it work for my use case involving autoCond updates. For the UL preparation, it is desirable to include finalized conditions in autoCond in master so that most of the updates are included in 11_0_X relval tests prior to their inclusion in 10_6_X. At the same time, there are PRs that include coupled code+conditions updates, and these should be tested separately from other condition updates. All of these PRs update the same lines in autoCond and so cannot be merged together. As backports are not merged as promptly as those in master, there may be three or four open PRs that modify the same line in autoCond.
In the scheme that I had in mind, your scheme would be written as, for example,
-
please test baseline #27651 with #27651
-
please test baseline #27651,#27698 with #27651,#27698
but that would not be sensible in the case of autoCond updates, in which global tags generally include all previous updates.
On another point: in your scheme one has to wait until the PR tests for the "earlier" PR(s) are completed prior to running the tests for the "later" PR. Since the tests take a non-negligible time to complete relative to the frequency of IB builds, the bot may use a different IB for the tests of the later PR. Could you instead ensure that when an alternate baseline is selected, the tests consistently use the same IB build?
@christopheralanwest , I am going to update bot which should allow us to generate baseline on demand (https://github.com/cms-sw/cms-bot/pull/1728 ). I think we can make use of this new feature to generate on demand baseline using extra PRs too e.g.
-
please test using baseline baseline-prs
-
please test with prs using baseline baseline-prs
can generate baseline using IB+baseline-prs
and use it to compare the results for IB+pr
or IB+pr+prs
. This also ensure that both baseline and PR tests use the same IB.
Question to you, do we still need this feature and will the above implementation is enough to cover your use case?
The feature requested in this PR would have been useful during the preparation for the Legacy production when new GT updates needed to be integrated before the PR including the previous set of updates had been merged. I don't know if GT changes are requested frequently enough nowadays that the feature is necessary. I'm no longer working on AlCa PRs so I don't need this feature myself, but I've added @cms-sw/alca-l2 to see if they would find it useful.
assign alca
- we'll need to digest a bit this before giving a coheren answer :)
New categories assigned: alca
@yuanchao,@francescobrivio,@malbouis,@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks