Catch renamed or removed dictionaries in PR tests

[fwyzard]

Looking at #51649 and #51756 I think that the current PR tests do not catch the case of types being removed or renamed, leaving behind missing dictionaries.

I would guess this happens because during PR tests the dictionaries are still available in the libraries from the original release.

Would it be possible to improve how scram b handles the removal of dictionaries, so these issues can be caught in a local area during development and in the PR tests ?

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @fwyzard.

@Dr15Jones, @ftenchini, @makortel, @mandrenguyen, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[fwyzard]

assign core

[cmsbuild]

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

[makortel]

Removed (or renamed) dictionaries should already be caught by PR tests.

I made a test in CMSSW_20_1_X_2026-08-21-1100 (last IB without #51649), commented out the dictionaries in SimDataFormats/Associations/src/classes_def.xml that were removed in #51649, and checked out Alignment/OfflineValidation. At least the DiElectronVertex test fails in the same way as reported in #51756.

Unfortunately the last tests #51649 (comment) have already been cleaned up, so I can't check if the Alignment/OfflineValidation tests were run as part of the PR or not. I'd guess they were not run.

So this issue appears to go in the bucket of "not all (unit) tests are run in PR tests".

[fwyzard]

I see.

I guess Alignment/OfflineValidation/test and any other tests that read pre-existing root files should be made to run any time there are changes to some DataFormats ?

[makortel]

I guess Alignment/OfflineValidation/test and any other tests that read pre-existing root files should be made to run any time there are changes to some DataFormats ?

I agree that would be nice. I'd guess that would require marking such tests in some way in the test/BuildFile.xml files. I believe in principle the tests could already declare a dependency to all DataFormat packages the files in those tests contain, but would that really feasible in most of the cases?

The "DataFormats" should probably cover all AnalysisDataFormats, DataFormats, FastSimDataFormats, SimDataFormats, TBDataFormats.

Additional question is then how much longer the PR tests would become.

In principle an alternative would be to build full CMSSW for all data format changes, but that sounds expensive.

@smuzaffar Any thoughts?

[smuzaffar]

@makortel , we can update bot to run full unit tests. We do not need to build full cmssw for this. After the PR build we can just checkout/symlink the rest of the cmssw and tell scram to run all tests without rebuild. This we do for IBs (where even a patch IB , with few packages, we still run all unit tests).

In IBs , full unit tests take 40-45mins. As for PRs, unit tests and Relvals run in parallel, so we should not see any delay in PR results but of course we will utilize more resources for PR test.

[makortel]

Running all unit tests for every PR would be even better, if we can afford it. I vaguely remember the last time this topic was discussed that was deemed to costly, and the full_cmssw option was added to the bot commands for the (anticipated) limited number of PRs where all tests are desired to be run.

It occurred to me that if we want to limit running all tests only to PRs that change data formats, we could utilize the changes-dataformats label.