Adding test for initial startup #849
Conversation
…ons raised are less likely to go uncaught
| standardSetup() | ||
| del standardSetup | ||
|
|
||
| def testStartUp(): |
There was a problem hiding this comment.
Does this function replicate the calls made in the "real" startup? If yes, is there a risk that it goes out of sync? Should this rather be factored out of the real startup such that the same code can be called here?
There was a problem hiding this comment.
@egede Yes and No. There are some changes and extra args which need to be passed here...
I'm exploring importing the ./bin/ganga file and using the code there I'm just exploring why this works when I test it but not in Jenkins atm. I can 'make' it work but I'm wondering if that's correct.
…ganga into testingIntialStartUp
|
OK, I've improved the test to not break things. I've also moved some common code out so that the code being tested is the code being used in the ganga executable during startup. |
|
I've also added some checks to see if the |
|
retest this please |
rob-c
force-pushed
the
testingIntialStartUp
branch
from
befac1e
to
6df907d
Compare
Fixing minor bug from corrections
| # Import GPI and run Ganga | ||
| from Ganga.GPI import * | ||
| setupGanga() | ||
| Ganga.Runtime._prog.run() |
There was a problem hiding this comment.
This didn't work for me without bringing back the import Ganga.Runtime
There was a problem hiding this comment.
How so? I'm curious what env causes problems as it could be a difference between my setup and LHCb vanilla
There was a problem hiding this comment.
Traceback (most recent call last):
File "/afs/cern.ch/user/m/masmith/cmtuser/GANGA/GANGA_HEAD/install/ganga/bin/ganga", line 62, in <module>
Ganga.Runtime._prog.run()
NameError: name 'Ganga' is not definedThere was a problem hiding this comment.
Hmm, looks like the standardSetup doesn't seem to take correctly and the test is over-compensating for this by inserting ganga into the python path. I'll see if I can make some changes to the standard startup script to fix this.
(I think we should really be using the __file__ property rather than the argv to determine where this file is...)
There was a problem hiding this comment.
Hmm, I'll try importing Ganga.Runtime then, strange how I can't seem to be able to reproduce this and the tests pass...
There was a problem hiding this comment.
Hmm, maybe I have done something peculiar to my setup...
There was a problem hiding this comment.
Does it work now? I decided to be explicit and import the _prog after it's been initialized
|
Spoke too soon - it has messed up something for creating jobs: Traceback (most recent call last):
File "/afs/cern.ch/user/m/masmith/cmtuser/GANGA/GANGA_HEAD/install/ganga/bin/ganga", line 63, in <module>
_prog.run()
File "/afs/cern.ch/user/m/masmith/cmtuser/GANGA/GANGA_HEAD/install/ganga/python/Ganga/Runtime/bootstrap.py", line 1058, in run
execfile(script, local_ns)
File "job.py", line 20, in <module>
j = Job()
NameError: name 'Job' is not defined |
|
@mesmith75 OK now I need to know how you reproduced this? I am able to launch an IPython prompt are you trying to do something different than use the interactive mode? |
|
@mesmith75 @rob-c This works for me. It starts Ganga and I can create a job. Tested both in a plain environment and in an LHCb environment. |
|
@egede I discovered this effects running scripts so I'm looking into it. |
|
OK, fixed that and solved the mystery of how user scripts were run from within the GPI. |
|
I'm afraid I still get errors, starting ganga with a job script: Traceback (most recent call last):
File "/afs/cern.ch/user/m/masmith/cmtuser/GANGA/GANGA_HEAD/install/ganga/bin/ganga", line 65, in <module>
log(Ganga.Runtime._prog.options.force_loglevel, err)
NameError: name 'Ganga' is not definedIt is fine to start ganga without the script argument |
Fixing _prog import in exceptions
|
@mesmith75 I get no problems running with a test job |
|
Thanks, that works fine. |
|
Any objections to merging this for 6.3.1? |
| except GangaException as err: | ||
| log(Ganga.Runtime._prog.options.force_loglevel, err) | ||
| from Ganga.Runtime import _prog |
There was a problem hiding this comment.
Why do we import this in two locations, within the try and the except rather than at the top?
There was a problem hiding this comment.
Hmm, I think your correct, this should be a single import
| logfile, maxBytes=logfile_size, backupCount=1) | ||
| with open(logfile, 'w'): | ||
| pass | ||
| new_file_handler = logging.handlers.RotatingFileHandler(logfile, maxBytes=logfile_size, backupCount=1) |
There was a problem hiding this comment.
Just a question, why do we need to open the file here and immediately close it?
There was a problem hiding this comment.
To guarantee that the file has been created. This seems to get rid of a lot of the problems that the rotating log file handler seems to have on one system where I was able to reproduce the problem that the stdout got spammed with lots of failed to flush to disk errors. It's a strange bug that is difficult to reproduce and this at least mitigates it. I'll add a doc string
|
Do we still want this in for 6.3.1? |
|
@mesmith75 Yes please if we can |
|
ok. If you could reply to @alexanderrichards review then we can stick it in. |
|
updated |
|
If we are happy, let's merge this and release the next version. |
This adds a small test which actually ensures that exceptions caused by launching Ganga in a clean env are caught. Most devs (myself included) don't often startup in a clean env regularly so it's possible to break things and for them to go un-noticed as we don't remove our .gangarc and similar files very often.