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

Fix FileInPath test in FWCore/PythonParameterSet #12376

Merged
merged 1 commit into from
Nov 12, 2015

Conversation

wmtan
Copy link
Contributor

@wmtan wmtan commented Nov 11, 2015

This needs to be tested as I cannot be 100% sure that this fixes the test, as I can't be certain that I've reproduced the conditions under which the test is run in the IB. Nonetheless, it should fix it.
There are two separate problems fixed by this PR. Problem 2) caused the test failure.

  1. The two local files used for the locality test were in a different package than the test. This will cause the test to fail if the package with the files is not checked out in the local working area. So, two dummy files were added to the test package for use in the locality test.
  2. The test actually did account for the possibility that it was being run from the base release. However, the test assumed that in the base release, CMSSW_RELEASE_BASE had been set to the empty string. This is, in fact, the case. However, if the test is run under scram, scram apparently sets CMSSW_RELEASE_BASE to the location of the base release. Because of this, the test does not recognize that it is being run from the base release, and tests fail. So this PR now also checks to see if CMSSW_RELEASE_BASE is equal to CMSSW_BASE, and also treats this as being run from the base release.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @wmtan for CMSSW_8_0_X.

Fix FileInPath test in FWCore/PythonParameterSet

It involves the following packages:

FWCore/ParameterSet
FWCore/PythonParameterSet

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' or '@cmsbuild, please test' 'please test with cms-sw/cmsdist#PR' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@wmtan
Copy link
Contributor Author

wmtan commented Nov 11, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9652/console

@cmsbuild
Copy link
Contributor

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

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@wmtan
Copy link
Contributor Author

wmtan commented Nov 11, 2015

@davidlange6 @Dr15Jones Since this problem occurs in all releases, please let me know to which releases, if any, this fix should be back ported.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Nov 12, 2015
Fix FileInPath test in FWCore/PythonParameterSet
@cmsbuild cmsbuild merged commit 8f9e9c3 into cms-sw:CMSSW_8_0_X Nov 12, 2015
@smuzaffar
Copy link
Contributor

@wmtan , looks like test is still failing in IB env. Here is what we do in IB to run unit tests, may be you try this too to check if it locally works for you

scram p CMSSW_8_0_X_2015-11-12-1100
cd CMSSW_8_0_X_2015-11-12-1100
cmsenv
rm -rf src
ln -s $CMSSW_RELEASE_BASE/src .
scram b -r echo_CXX
set XX=`echo "$CMSSW_RELEASE_BASE/test/$SCRAM_ARCH "``scram tool info cmssw | grep CMSSW_BASE= | sed 's|.*CMSSW_BASE=||'`
setenv PATH `echo $XX | tr ' ' ':'`:$PATH
scram build -f -k unittests_testPythonParameterSet
scram b -f -k unittests_testPythonParameterSet

@wmtan
Copy link
Contributor Author

wmtan commented Nov 12, 2015

@smuzaffar Yes, I have reproduced this using your script. I'll investigate.

@smuzaffar
Copy link
Contributor

The above receipe will not take much time. It is a way for running unit tests from the release area without re-building any thing locally.
For IBs, we just create dev area, create symlink for $CMSSW_RELEASE_BASE/src and run "scram build -r echo_CXX" which basically do not compile any thing but just re build the internal scram caches ( so that scram can know which tests are available and how to run those).

@wmtan
Copy link
Contributor Author

wmtan commented Nov 12, 2015

@smuzaffar Unfortunately, although I can reproduce the error, I cannot debug it. I cannot edit the test on a CERN machine. If I set this up on my own machine, I can still reproduce the error, but any changes I make in the test do not affect the test. It always runs the test exactly as it is in the IB, and does not see my modifications.
I need a way to reproduce this so that I can edit the code and have the changes affect the test.

@wmtan wmtan deleted the FileInPathFix80X branch November 16, 2015 17:38
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.

5 participants