Additional Unit test Fix for 12107

[todor-ivanov]

Fixes #12037

Status

ready

Description

With PR #12108 we forgot to fix one Unit test and remove tow commented lines. The current PR addresses this.

Is it backward compatible (if not, which system it affects?)

YES

Related PRs

#12108

External dependencies / deployment changes

No

[todor-ivanov]

@amaltaro this is all fixed here. I am merging this PR so that others do not wait for their broken unit tests. Please leave any comments in a post merge the review if you have any.

[amaltaro]

@todor-ivanov did you merge it by mistake? I don't understand why you would not wait for Jenkins CI to do its job and come back with the result(!)

[todor-ivanov]

hi @amaltaro no I did not merge it by mistake ... .it was a simple fix and the tests were successful, and I did not want to stop other peoples' work with a failing unit test.

[todor-ivanov]

Because I have no idea why would jenkins claim it as failed... since this was exactly the test that this PR fixes and I have tested even before I created the PR. Here is what I get in my local run for this unit test:

(WMAgent.venv3) cmst1@vocms0290:WMAgent.venv3 $ ipython -m unittest  srv/WMCore/test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py 
%reload_ext autoreload
%autoreload 2
3{'RequestPriority': 0}
{'RequestPriority': 100}
{'RequestPriority': 999999}
..reqArgsDiff: {'RequestPriority': None}
expectedReqArgs: {'RequestPriority': None}
===============================
reqArgsDiff: {'RequestPriority': None, 'Team': None, 'AcquisitionEra': None, 'ProcessingString': None, 'ProcessingVersion': None, 'SiteBlacklist': None, 'SiteWhitelist': None, 'CustodialSites': None, 'NonCustodialSites': None, 'SubscriptionPriority': None, 'UnmergedLFNBase': None, 'MergedLFNBase': None, 'MinMergeSize': None, 'MaxMergeSize': None, 'MaxMergeEvents': None, 'TrustSitelists': None, 'TrustPUSitelists': None, 'SoftTimeout': None, 'GracePeriod': None, 'BlockCloseMaxWaitTime': None, 'BlockCloseMaxFiles': None, 'BlockCloseMaxEvents': None, 'BlockCloseMaxSize': None, 'Dashboard': None, 'Override': None}
expectedReqArgs: {'RequestPriority': None, 'Team': None, 'SiteWhitelist': None, 'SiteBlacklist': None, 'AcquisitionEra': None, 'ProcessingString': None, 'ProcessingVersion': None, 'Dashboard': None, 'MergedLFNBase': None, 'TrustSitelists': None, 'UnmergedLFNBase': None, 'MinMergeSize': None, 'MaxMergeSize': None, 'MaxMergeEvents': None, 'BlockCloseMaxWaitTime': None, 'BlockCloseMaxFiles': None, 'BlockCloseMaxEvents': None, 'BlockCloseMaxSize': None, 'SoftTimeout': None, 'GracePeriod': None, 'TrustPUSitelists': None, 'CustodialSites': None, 'NonCustodialSites': None, 'Override': None, 'SubscriptionPriority': None}
===============================
reqArgsDiff: {'RequestPriority': None}
expectedReqArgs: {'RequestPriority': None}
===============================
reqArgsDiff: {'RequestPriority': None, 'SiteBlacklist': None, 'SiteWhitelist': None}
expectedReqArgs: {'RequestPriority': None, 'SiteWhitelist': None, 'SiteBlacklist': None}
===============================
reqArgsDiff: {'RequestPriority': None}
expectedReqArgs: {'RequestPriority': None}
===============================
reqArgsDiff: {'RequestPriority': None, 'SiteBlacklist': None, 'SiteWhitelist': None}
expectedReqArgs: {'RequestPriority': None, 'SiteWhitelist': None, 'SiteBlacklist': None}
===============================
reqArgsDiff: {'RequestPriority': None, 'SiteBlacklist': None, 'SiteWhitelist': None}
expectedReqArgs: {'RequestPriority': None, 'SiteWhitelist': None, 'SiteBlacklist': None}
===============================
reqArgsDiff: {'RequestPriority': None}
expectedReqArgs: {'RequestPriority': None}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
reqArgsDiff: {}
expectedReqArgs: {}
===============================
.
----------------------------------------------------------------------
Ran 3 tests in 0.093s

OK

[todor-ivanov]

and you can also see this very same test have changed from failure to success in the PR which @anpicci is working:
(which was actually the initial request, because of which we noticed we have missed this one here):
https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15239/artifact/artifacts/PullRequestReport.html#pylintpy3

Here is the report from Andrea's PR jenkins actions, which I triggered upon merging this test:

WMCore_t.ReqMgr_t.Utils_t.Validation_t.ValidationTests:testValidateRequestAllowedArgs changed from failure to success

I do not think we have anything to do here. I only wonder if we can force Jenkins to run the tests again on an already merged PR ....

[amaltaro]

@todor-ivanov now it has been merged already and we know everything is working as expected, as you retrieved this information from Andrea.

For future occasions though, please give Jenkins the 15-20min to run its job and do not merge anything before Jenkins tests come back and usual tests are successful. We know that it is not 100% of the time that Jenkins and localhost unit tests match, so it's always wise to wait for Jenkins.