Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Run custom CMS clang-tidy checks in PR testing. #1514

Merged
merged 8 commits into from
Mar 29, 2021
Merged

Run custom CMS clang-tidy checks in PR testing. #1514

merged 8 commits into from
Mar 29, 2021

Conversation

gartung
Copy link
Member

@gartung gartung commented Mar 17, 2021 •
edited
Loading

Alternative way to run EventSetupRecord::get checker.
Resolves cms-sw/framework-team#69

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gartung (Patrick Gartung) for branch master.

@cmsbuild, @smuzaffar, @mrodozov can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

@gartung gartung mentioned this pull request Mar 17, 2021
Merged
@smuzaffar
Copy link
Contributor

please test with cms-sw/cmssw#33186

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0f0e0d/13582/summary.html
COMMIT: 2b4474c
CMSSW: CMSSW_11_3_X_2021-03-17-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cms-bot/1514/13582/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639935
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2639900
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@gartung
Copy link
Member Author

gartung commented Mar 18, 2021

@makortel The summary shows that clang-tidy was run on the PR as expected.

@makortel
Copy link
Contributor

If the log (which I presume is this one) would have warnings on EventSetupRecord::get() on the files touched by the PR, would they be shown in the summary message #1514 (comment)? (otherwise, I'm afraid, it wouldn't add much value to the static analyzer that we already run)

By the way, what is the difference between "CMS Code checks outputs" and "Static checks outputs"? They both seem to point to the static analyzer report.

@gartung
Copy link
Member Author

gartung commented Mar 18, 2021

OK. This is a draft PR. The runCMSCodeChecks.log is only supposed to show the output of clang-tidy. The summary page is messed up because I used the same key as Shahzad indicated above.

@cmsbuild
Copy link
Contributor

Pull request #1514 was updated.

3 similar comments
@cmsbuild
Copy link
Contributor

Pull request #1514 was updated.

@cmsbuild
Copy link
Contributor

Pull request #1514 was updated.

@cmsbuild
Copy link
Contributor

Pull request #1514 was updated.

@gartung
Copy link
Member Author

gartung commented Mar 18, 2021

please test with cms-sw/cmssw#33186

@cmsbuild
Copy link
Contributor

Pull request #1514 was updated.

@gartung
Copy link
Member Author

gartung commented Mar 29, 2021

please test with cms-sw/cmssw#33215

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0f0e0d/13828/summary.html
COMMIT: d5630c8
CMSSW: CMSSW_11_3_X_2021-03-28-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cms-bot/1514/13828/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are 1 Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0f0e0d/13828/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1284 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639935
  • DQMHistoTests: Total failures: 3675
  • DQMHistoTests: Total nulls: 20
  • DQMHistoTests: Total successes: 2636218
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 45.699 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 140.53 ): 44.531 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): 1.172 KiB RPC/DCSInfo
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@smuzaffar
Copy link
Contributor

please test for CMSSW_11_1_X
lets test if not an old release cycle

@smuzaffar
Copy link
Contributor

please test with cms-sw/cmssw#33153 for CMSSW_10_6_X

@smuzaffar
Copy link
Contributor

looks good

@smuzaffar
Copy link
Contributor

+externals

@smuzaffar smuzaffar merged commit 926833f into cms-sw:master Mar 29, 2021
@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0f0e0d/13834/summary.html
COMMIT: d5630c8
CMSSW: CMSSW_11_1_X_2021-03-28-0000/slc7_amd64_gcc820
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cms-bot/1514/13834/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1264 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2784828
  • DQMHistoTests: Total failures: 1984
  • DQMHistoTests: Total nulls: 44
  • DQMHistoTests: Total successes: 2782750
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 45.703 KiB( 35 files compared)
  • DQMHistoSizes: changed ( 140.53 ): 44.531 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): 1.172 KiB RPC/DCSInfo
  • Checked 152 log files, 31 edm output root files, 36 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0f0e0d/13835/summary.html
COMMIT: d5630c8
CMSSW: CMSSW_10_6_X_2021-03-29-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cms-bot/1514/13835/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: 35
  • DQMHistoTests: Total histograms compared: 3215541
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215206
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 143 log files, 29 edm output root files, 35 DQM output files
  • TriggerResults: no differences found

@gartung gartung deleted the gartung-pr-check-cc-warning branch April 1, 2021 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smuzaffar smuzaffar smuzaffar left review comments

Assignees
No one assigned
Labels
externals-approved fully-signed orp-pending requires-external tests-approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add clang-tidy checker that flags use of EventSetupRecord::get()
4 participants
@gartung @cmsbuild @smuzaffar @makortel