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.

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

add unit tests for getPayloadData.py #34199

Merged
merged 1 commit into from Jun 25, 2021

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Jun 21, 2021

PR description:

This is a follow-up to issue #34193.
Added a unit test to avoid having to find post-facto that the getPayloadData script employed by the CondDB Browser is broken.

PR validation:

Tried to run the new uni test, which fails as reported at #34193 (comment).
The commands included in CondCore/Utilities/test/test_getPayloadData.sh were successfully executed in CMSSW_12_0_0_pre2.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Not a backport, no backport needed.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34199/23421

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

CondCore/Utilities

@ggovi, @cmsbuild can you please review it and eventually sign? Thanks.
@mmusich this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@qliphy
Copy link
Contributor

qliphy commented Jun 22, 2021

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f5f44a/16151/summary.html
COMMIT: a955dfb
CMSSW: CMSSW_12_0_X_2021-06-21-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34199/16151/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 testGetPayloadData had ERRORS

Comparison Summary

Summary:

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

@mmusich mmusich changed the title modify getPayloadData to comply with python3 and add unit tests add unit tests for getPayloadData.py Jun 22, 2021
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34199/23426

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

Pull request #34199 was updated. @ggovi, @cmsbuild can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #34199 was updated. @ggovi, @cmsbuild can you please check and sign again.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 23, 2021

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f5f44a/16193/summary.html
COMMIT: 50c6bd6
CMSSW: CMSSW_12_0_X_2021-06-23-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34199/16193/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2785631
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2785602
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor Author

mmusich commented Jun 25, 2021

@ggovi any (further) objection to this PR?

@ggovi
Copy link
Contributor

ggovi commented Jun 25, 2021

+1

@cmsbuild
Copy link
Contributor

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Jun 25, 2021

+1

@cmsbuild cmsbuild merged commit 5d92665 into cms-sw:master Jun 25, 2021
@mmusich mmusich deleted the getPayloadDataUnitTest branch June 25, 2021 20:45
@qliphy
Copy link
Contributor

qliphy commented Jun 26, 2021

@mmusich There is a unit failure in most recent IB after this PR is merged. Can you have a check? Thanks!

https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-06-25-2300/unitTestLogs/CondCore/Utilities#/181-181
Traceback (most recent call last):
File "", line 1, in
KeyError: 'cond::BasicPayload'
getPayloadData.py --discover test not passed... found no entries

@mmusich
Copy link
Contributor Author

mmusich commented Jun 26, 2021

@qliphy I cannot reproduce the issue:

cmsrel CMSSW_12_0_X_2021-06-25-2300
cd CMSSW_12_0_X_2021-06-25-2300/src/
cmsenv
git cms-addpkg CondCore/Utilities
scram b -j 20
scram b runtests_testGetPayloadData
...

---> test testGetPayloadData succeeded
TestTime:80
^^^^ End Test testGetPayloadData ^^^^

isn't the package compiled before executing the unit test in the IB?

@smuzaffar
Copy link
Contributor

@mmusich , in IB getPayloadData.py --discover returned empty results ( i.e. {} ) e.g.

scram p CMSSW_12_0_X_2021-06-25-2300
cd CMSSW_12_0_X_2021-06-25-2300
cmsenv
getPayloadData.py --discover

could it be that the https://github.com/cms-sw/cmssw/blob/master/CondCore/CondDB/plugins/CondDBPyWrappers.cc#L11-L14 is looking in to only local edmplugin information? Note that for IB unit tests we just create a local dev area and run without re-compilation , so every thing should be picked up from release area.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 26, 2021

@smuzaffar so substantially my question about:

isn't the package compiled before executing the unit test in the IB?

is no.
Anyway I've already found that out, see #34257

@smuzaffar
Copy link
Contributor

smuzaffar commented Jun 26, 2021

I think problem is https://github.com/cms-sw/cmssw/blob/master/CondCore/Utilities/scripts/getPayloadData.py#L140 , it breaks on first release path. I think it should go through all the paths (specially if there are no plugins found ). As I wrote, for IBs, we just create an empty cmssw area, so the lib direcotry in that will be empty.

Please also add CMSSW_FULL_RELEASE_BASE in the release list https://github.com/cms-sw/cmssw/blob/master/CondCore/Utilities/scripts/getPayloadData.py#L125 . This will allow the script to work if it is running in a patch release

@smuzaffar
Copy link
Contributor

ah thanks for #34257 :-) please also add CMSSW_FULL_RELEASE_BASE

@qliphy
Copy link
Contributor

qliphy commented Jul 16, 2021

See #34512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants