Remove use of legacy module classes from framework tests #35526

Dr15Jones · 2021-10-04T16:35:53Z

PR description:

replaced use of legacy module types with thread friendly types
any remaining ones were left intentionally until we remove those APIs

PR validation:

Built using USER_CXXFLAGS='-DUSE_CMS_DEPRECATED' scram b -j ...

cmsbuild · 2021-10-04T16:42:32Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35526/25737

This PR adds an extra 84KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Integration/test/TestParentage.cc modified in PR(s): Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326
- File IOPool/SecondaryInput/test/SecondaryProducer.h modified in PR(s): Use StatisticsSenderService for all framework files #35505

cmsbuild · 2021-10-04T16:42:57Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Framework (core)
FWCore/Integration (core)
FWCore/MessageService (core)
FWCore/Modules (core)
FWCore/Services (core)
IOPool/SecondaryInput (core)
IOPool/Streamer (core)

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @wddgit this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2021-10-04T16:43:31Z

please test

cmsbuild · 2021-10-04T19:59:07Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19379/summary.html
COMMIT: ea9139b
CMSSW: CMSSW_12_1_X_2021-10-04-1300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35526/19379/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestIntegrationGetBy had ERRORS
---> test TestIntegrationParameterSet had ERRORS
---> test TestIntegrationRunMerge had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3219394
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3219366
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

makortel · 2021-10-04T20:03:16Z

Given the errors (I didn't look), how about we go with the #35326 version of TestParentage?

wddgit · 2021-10-04T21:41:47Z

If you want me to look at this I will. I think I wrote the merge testing code years ago. The last failure is an assert. It might take a while to understand that one. The other two look like reference files that need updating, which is simple assuming the changes are expected.

cmsbuild · 2021-10-04T22:27:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35526/25747

This PR adds an extra 104KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/Integration/test/TestParentage.cc modified in PR(s): Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326
- File IOPool/SecondaryInput/test/SecondaryProducer.h modified in PR(s): Use StatisticsSenderService for all framework files #35505

cmsbuild · 2021-10-04T22:27:49Z

Pull request #35526 was updated. @makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please check and sign again.

Dr15Jones · 2021-10-04T22:57:50Z

please test

cmsbuild · 2021-10-05T14:47:08Z

Pull request #35526 was updated. @makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please check and sign again.

Dr15Jones · 2021-10-05T14:47:32Z

please test

cmsbuild · 2021-10-05T18:32:46Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19415/summary.html
COMMIT: 277322d
CMSSW: CMSSW_12_1_X_2021-10-05-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35526/19415/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@tvami Remove DQMReferenceHistogramRootFileRcd from all GTs #35533
@kbunkow new package L1Trigger/L1TMuonOverlapPhase1 (new version of the L1 Trigger Overlap Muon Track Finder (OMTF) emulator) #35111

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19415/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19415/git-merge-result

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3219394
DQMHistoTests: Total failures: 88
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3219284
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

makortel · 2021-10-05T19:03:57Z

@cmsbuild, please test

For cleaner comparisons

cmsbuild · 2021-10-05T22:42:50Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19419/summary.html
COMMIT: 277322d
CMSSW: CMSSW_12_1_X_2021-10-05-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35526/19419/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@tvami Remove DQMReferenceHistogramRootFileRcd from all GTs #35533
@kbunkow new package L1Trigger/L1TMuonOverlapPhase1 (new version of the L1 Trigger Overlap Muon Track Finder (OMTF) emulator) #35111

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19419/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19419/git-merge-result

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3219394
DQMHistoTests: Total failures: 83
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3219289
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

makortel · 2021-10-06T15:02:36Z

@cmsbuild, please test

(for cleaner comparisons now that the new IB is actually there)

cmsbuild · 2021-10-06T18:15:29Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8217ee/19440/summary.html
COMMIT: 277322d
CMSSW: CMSSW_12_1_X_2021-10-06-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35526/19440/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3219394
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3219372
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

makortel · 2021-10-06T18:22:32Z

+1

cmsbuild · 2021-10-06T18:23:01Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

perrotta · 2021-10-07T17:23:37Z

+1

cmsbuild added this to the CMSSW_12_1_X milestone Oct 4, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Oct 4, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 4, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 4, 2021

cmsbuild mentioned this pull request Oct 4, 2021

Throw an exception if a module requiring synchronization on lumi boundaries is used when concurrent lumis are enabled #35326

Merged

cmsbuild added tests-rejected and removed tests-started labels Oct 4, 2021

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Oct 4, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 4, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 4, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 5, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 5, 2021

cmsbuild added tests-started and removed tests-approved labels Oct 5, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 5, 2021

cmsbuild added tests-started and removed tests-approved labels Oct 6, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 6, 2021

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Oct 6, 2021

cmsbuild added orp-approved and removed orp-pending labels Oct 7, 2021

cmsbuild merged commit 8c64ddc into cms-sw:master Oct 7, 2021

Dr15Jones deleted the fixDeprecatedFramework branch October 20, 2021 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove use of legacy module classes from framework tests #35526

Remove use of legacy module classes from framework tests #35526

Dr15Jones commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

Dr15Jones commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

makortel commented Oct 4, 2021

wddgit commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

Dr15Jones commented Oct 4, 2021

cmsbuild commented Oct 5, 2021

Dr15Jones commented Oct 5, 2021

cmsbuild commented Oct 5, 2021

makortel commented Oct 5, 2021

cmsbuild commented Oct 5, 2021

makortel commented Oct 6, 2021

cmsbuild commented Oct 6, 2021

makortel commented Oct 6, 2021

cmsbuild commented Oct 6, 2021

perrotta commented Oct 7, 2021

Remove use of legacy module classes from framework tests #35526

Remove use of legacy module classes from framework tests #35526

Conversation

Dr15Jones commented Oct 4, 2021

PR description:

PR validation:

cmsbuild commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

Dr15Jones commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

Unit Tests

Comparison Summary

makortel commented Oct 4, 2021

wddgit commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

cmsbuild commented Oct 4, 2021

Dr15Jones commented Oct 4, 2021

cmsbuild commented Oct 5, 2021

Dr15Jones commented Oct 5, 2021

cmsbuild commented Oct 5, 2021

Comparison Summary

makortel commented Oct 5, 2021

cmsbuild commented Oct 5, 2021

Comparison Summary

makortel commented Oct 6, 2021

cmsbuild commented Oct 6, 2021

Comparison Summary

makortel commented Oct 6, 2021

cmsbuild commented Oct 6, 2021

perrotta commented Oct 7, 2021