Merge into trunk : ipython-py2exe-fix : Code : IPython

Reviewer	Review Type	Date Requested	Status
Fernando Perez		2008-12-07	Needs Fixing on 2008-12-08
Review via email: mp+2190@code.launchpad.net

Revision history for this message

Jörgen Stenarson (jorgen-stenarson) wrote on 2008-12-07:

#

This patch branch was created to fix the problem in Bug #274067.
Where genutils.get_homedir doesn't work with py2exe when not using a
zip-compressed library. The problem was the approach used to
detect when running under py2exe. The old check was if IPython.__file__
contained library.zip which won't work if py2exe is used without a
compressed library, a better way is to look for 'frozen' in sys.

During the work on this patch I also did changes to

genutils.get_home_dir():

- changed the if statement for py2exe detection

- Added KeyError exception to one bare except statement.

Remaining issues:
- There is still one bare exception in the function

- Is it a good idea to return 'C:\\' as a path if no
user path was found? In my mind it is basically like
returning root on a unix machine which is a not a good idea.

genutils.get_ipyton_dir():
- change to return unicode path, otherwise pre_config_initialization
in iplib failed with UnicodeError for path containing non ascii letters.

ipmaker.make_IPython:

- replaced code with call to get_ipyton_dir

tests/test_genutils:

- extended test for get_home_dir. I have created a directory tree
   (home_test_dir, home_test_dir/_ipython) that I use in these tests
   to have a known good place to use for HOME and IPYTHONDIR. On
   non windows platforms I create a dummy module for _winreg to allow
   all tests to run on all platforms.

- extended test for get_ipyton_dir

- extended test for get_security_dir

- added tests for popkey

Revision history for this message

Fernando Perez (fdo.perez) wrote on 2008-12-08:

#

This looks mostly OK. I'd make a few minor fixes before committing though.

* 1148: test_get_home_dir2 might as well just replace the original one, since it does the call anyway. That old test was there mostly as a placeholder, because even a no-check test that calls something is better than no test. But once you write a real test, it's OK to replace that overly simplistic one.

- It's a good idea to use either setup/teardown or try/finally when you change things like os.environ['HOME']. Otherwise if the test breaks, it will leave a seriously damaged environment that will likely cause other weird problems later. nose allows per-function setup/teardown functions if you want to use that, or just a plain try/finally is OK too.

Note: I see that later revisions use a setup decorator. OK...

- Don't use plain 'assert' in tests, use one of the nose.tools.assert* functions. They have better error reporting and don't get optimized out of the bytecode by the compiler. Tests basically should never use the naked assert keyword. For example, use:

import nose.tools as nt
nt.assert_equal(home_dir,env['HOME'])

*1149: for checking exceptions, use nt.assert_raises, which is shorter and clearer and does the same thing.

- The home_test_dir directory should be created dynamically during testing, not added as a hard path in the repo.

* 1151: skip_if* decorators should be put in testing/decorators.py so they can be more easily reused.

- the env setup/teardown decorator functions should have docstrings so we know what they're meant to do. Even if they are just an internal part of the test suite, ALL functions should have a docstring so we know what they are supposed to do.

* 1153: a bit of extraneous whitespace in test_genutils (too many blank lines around line 62)

* 1154: the regexp for shlash_prefilter_f should be compiled statically and cached for performance reasons. prefiltering is on the critical path, so it should be as fast as possible.

* General:

- please stick to spaces around assignments/equality checks ('a = 1', 'a == b') everywhere except for keyword args to functions (foo(bar=True) is OK). This is just pep 8 stuff (see "Whitespace in Expressions and Statements" section, we mostly follow pep 8 across the board).

** Summary **
Great patch all around, and many thanks for putting proper tests and docs everywhere! I think one round of easy updates will leave this ready for commit.

This looks mostly OK.  I'd make a few minor fixes before committing though.

* 1148: test_get_home_dir2 might as well just replace the original one, since it does the call anyway.  That old test was there mostly as a placeholder, because even a no-check test that calls something is better than no test.  But once you write a real test, it's OK to replace that overly simplistic one.

- It's a good idea to use either setup/teardown or try/finally when you change things like os.environ['HOME'].  Otherwise if the test breaks, it will leave a seriously damaged environment that will likely cause other weird problems later.  nose allows per-function setup/teardown functions if you want to use that, or just a plain try/finally is OK too.

Note: I see that later revisions use a setup decorator.  OK...

- Don't use plain 'assert' in tests, use one of the nose.tools.assert* functions.  They have better error reporting and don't get optimized out of the bytecode by the compiler.  Tests basically should never use the naked assert keyword.  For example, use:

import nose.tools as nt
  nt.assert_equal(home_dir,env['HOME'])

*1149: for checking exceptions, use nt.assert_raises, which is shorter and clearer and does the same thing.

- The home_test_dir directory should be created dynamically during testing, not added as a hard path in the repo.

* 1151: skip_if* decorators should be put in testing/decorators.py so they can be more easily reused.

- the env setup/teardown decorator functions should have docstrings so we know what they're meant to do.  Even if they are just an internal part of the test suite, ALL functions should have a docstring so we know what they are supposed to do.

* 1153: a bit of extraneous whitespace in test_genutils (too many blank lines around line 62)

* 1154: the regexp for shlash_prefilter_f should be compiled statically and cached for performance reasons.  prefiltering is on the critical path, so it should be as fast as possible.

* General:

- please stick to spaces around assignments/equality checks ('a = 1', 'a == b') everywhere except for keyword args to functions (foo(bar=True) is OK).  This is just pep 8 stuff (see "Whitespace in Expressions and Statements" section, we mostly follow pep 8 across the board).

** Summary ** 
Great patch all around, and many thanks for putting proper tests and docs everywhere!  I think one round of easy updates will leave this ready for commit.

review: Needs Fixing

lp:~jorgen-stenarson/ipython/ipython-py2exe-fix updated on 2008-12-09

1157. By Jörgen Stenarson on 2008-12-09: Fixed bug on tests so they work when iptest is not called from within IPython/tests
1158. By Jörgen Stenarson on 2008-12-09: Removing simple test cases for get_home_dir and get_ipython_dir
1159. By Jörgen Stenarson on 2008-12-09: Fixing pep-8 conformance issues
1160. By Jörgen Stenarson on 2008-12-09: Remove bare asserts by switching to use nose.tools.assert_* functions to check test conditions.
1161. By Jörgen Stenarson on 2008-12-09: Moved skip decorator to testing and created similar ones for OSX and linux, create delete testdirs in module setup/teardown
1162. By Jörgen Stenarson on 2008-12-09: Some white space fixing, and comments
1163. By Jörgen Stenarson on 2008-12-09: Add doc strings to all functions
1164. By Jörgen Stenarson on 2008-12-09: Added a python3.0 porting section to the development.txt document

Revision history for this message

Jörgen Stenarson (jorgen-stenarson) wrote on 2008-12-09:

#

Thanks for the comments. I have tried to address all the issues.

1149: I have added dynamic creation of the testdir, is it ok to use hardcoded filenames or should dynamic ones be dynamically created? I ignore any errors on creation/deletion here is that ok?

1151: I moved the skip_if decorator to testing/decorators.py and created ones for linux and osx.
Now I'm not using this in my patch instead I have stubbed the functions in _winreg so the tests hopefully work on linux etc. This has not been verified by me so I would appreciate if someone could run it on linux and make sure it works.

1154: is just a merge from trunk and not part of my patch.

I have added a python 3.0 porting section last in development.txt

Revision history for this message

Fernando Perez (fdo.perez) wrote on 2009-02-01:

#

Download full text (6.4 KiB)

Hi Jorgen,

On Tue, Dec 9, 2008 at 11:24 AM, Jörgen Stenarson
<email address hidden> wrote:

> 1149: I have added dynamic creation of the testdir, is it ok to use hardcoded filenames or should dynamic ones be dynamically created? I ignore any errors on creation/deletion here is that ok?
>
> 1151: I moved the skip_if decorator to testing/decorators.py and created ones for linux and osx.
> Now I'm not using this in my patch instead I have stubbed the functions in _winreg so the tests hopefully work on linux etc. This has not been verified by me so I would appreciate if someone could run it on linux and make sure it works.
>
> 1154: is just a merge from trunk and not part of my patch.
>
> I have added a python 3.0 porting section last in development.txt

For the most part I think this is good to go, I really appreciate your
taking my previous comments to heart, and an apology again for
vanishing in the last few weeks.

However, in its current status several things break:

======================================================================
ERROR: Testcase $HOME is set, then use its value as home directory.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 132, in test_get_home_dir_3
    home_dir = genutils.get_home_dir()
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
line 949, in get_home_dir
    raise HomeDirError,'undefined $HOME, IPython can not proceed.'
HomeDirError: undefined $HOME, IPython can not proceed.

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 150, in test_get_home_dir_5
    del os.environ["HOME"]
  File "/usr/lib/python2.5/os.py", line 499, in __delitem__
    del self.data[key]
KeyError: 'HOME'

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 164, in test_get_home_dir_6
    del os.environ["HOME"]
  File "/usr/lib/python2.5/os.py", line 499, in __delitem__
    del self.data[key]
KeyError: 'HOME'

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
...

Hi Jorgen,

On Tue, Dec 9, 2008 at 11:24 AM, Jörgen Stenarson
<jorgen.stenarson@bostream.nu> wrote:

> 1149: I have added dynamic creation of the testdir, is it ok to use hardcoded filenames or should dynamic ones be dynamically created? I ignore any errors on creation/deletion here is that ok?
>
> 1151: I moved the skip_if decorator to testing/decorators.py and created ones for linux and osx.
> Now I'm not using this in my patch instead I have stubbed the functions in _winreg so the tests hopefully work on linux etc. This has not been verified by me so I would appreciate if someone could run it on linux and make sure it works.
>
> 1154: is just a merge from trunk and not part of my patch.
>
> I have added a python 3.0 porting section last in development.txt

For the most part I think this is good to go, I really appreciate your
taking my previous comments to heart, and an apology again for
vanishing in the last few weeks.

However, in its current status several things break:

======================================================================
ERROR: Testcase $HOME is set, then use its value as home directory.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 132, in test_get_home_dir_3
    home_dir = genutils.get_home_dir()
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
line 949, in get_home_dir
    raise HomeDirError,'undefined $HOME, IPython can not proceed.'
HomeDirError: undefined $HOME, IPython can not proceed.

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 150, in test_get_home_dir_5
    del os.environ["HOME"]
  File "/usr/lib/python2.5/os.py", line 499, in __delitem__
    del self.data[key]
KeyError: 'HOME'

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 164, in test_get_home_dir_6
    del os.environ["HOME"]
  File "/usr/lib/python2.5/os.py", line 499, in __delitem__
    del self.data[key]
KeyError: 'HOME'

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 179, in test_get_home_dir_7
    del env["HOME"], env['HOMEDRIVE']
  File "/usr/lib/python2.5/os.py", line 499, in __delitem__
    del self.data[key]
KeyError: 'HOME'

======================================================================
ERROR: Testcase to see if we can call get_security_dir without Exceptions.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 233, in test_get_security_dir
    sdir = genutils.get_security_dir()
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
line 1012, in get_security_dir
    os.mkdir(security_dir, 0700)
OSError: [Errno 2] No such file or directory:
'/home/fperez/someplace/_ipython/security'

======================================================================
FAIL: Testcase for py2exe logic, un-compressed lib
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 115, in test_get_home_dir_1
    nt.assert_equal(home_dir, abspath(join(test_file_path, "home_test_dir")))
AssertionError: '/home/fperez' !=
'/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'
>>  raise self.failureException, \
          (None or '%r != %r' % ('/home/fperez',
'/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'))

======================================================================
FAIL: Testcase for py2exe logic, compressed lib
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 126, in test_get_home_dir_2
    nt.assert_equal(home_dir, abspath(join(test_file_path,
"home_test_dir")).lower())
AssertionError: '/home/fperez' !=
'/home/fperez/usr/lib/python2.5/site-packages/ipython/tests/home_test_dir'
>>  raise self.failureException, \
          (None or '%r != %r' % ('/home/fperez',
'/home/fperez/usr/lib/python2.5/site-packages/ipython/tests/home_test_dir'))

----------------------------------------------------------------------
Ran 431 tests in 43.216s

FAILED (SKIP=4, errors=5, failures=2)

These should be fairly easy to fix though.  Do not delete env['HOME']
unconditionally, since it will mess other things up (at least in
linux).

Let me know if you need ideas, otherwise just ping me for further
testing.  Once the tests pass for both you and me, I'd say go ahead
with the merge, unless anyone else has any other review comments.

Great job, thanks.

f

lp:~jorgen-stenarson/ipython/ipython-py2exe-fix updated on 2009-02-01

1165. By Jörgen Stenarson on 2009-02-01: Test for presence before deleting keys from os.environ. Mark two tests
for skipping when not on win32 platform.

Revision history for this message

Jörgen Stenarson (jorgen-stenarson) wrote on 2009-02-01:

#

Hi Fernando,

I think I have fixed the failures. I have tested on a Ubuntu machine
successfully.

The del env['HOME'] are deliberate because I need to test the code paths
in the get_ functions when HOME is missing. However there were cases
when I tried to delete it several times which doesn't work. So now I
have added tests so it is only deleted when present.

I also marked two tests as windows only.

/Jörgen

Revision history for this message

Jörgen Stenarson (jorgen-stenarson) wrote on 2009-02-01:

#

Hi Fernando,

I think I have fixed the failures. I have tested on a Ubuntu machine successfully.

The del env['HOME'] are deliberate because I need to test the code paths in the get_ functions when HOME is missing. However there were cases when I tried to delete it several times which doesn't work. So now I have added tests so it is only deleted when present.

I also marked two tests as windows only.

/Jörgen

Revision history for this message

Fernando Perez (fdo.perez) wrote on 2009-02-01:

#

Download full text (3.4 KiB)

Hi Jorgen,

On Sun, Feb 1, 2009 at 7:18 AM, Jörgen Stenarson
<email address hidden> wrote:
> Hi Fernando,
>
> I think I have fixed the failures. I have tested on a Ubuntu machine
> successfully.

Mmh, I'm still getting errors and failures:

======================================================================
ERROR: Testcase $HOME is set, then use its value as home directory.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 132, in test_get_home_dir_3
    home_dir = genutils.get_home_dir()
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
line 949, in get_home_dir
    raise HomeDirError,'undefined $HOME, IPython can not proceed.'
HomeDirError: undefined $HOME, IPython can not proceed.

======================================================================
ERROR: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 168, in test_get_home_dir_6
    home_dir = genutils.get_home_dir()
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
line 957, in get_home_dir
    raise HomeDirError
HomeDirError

======================================================================
FAIL: Testcase for py2exe logic, un-compressed lib
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 114, in test_get_home_dir_1
    nt.assert_equal(home_dir, abspath(join(test_file_path, "home_test_dir")))
AssertionError: '/home/fperez' !=
'/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'
>> raise self.failureException, \
          (None or '%r != %r' % ('/home/fperez',
'/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'))

======================================================================
FAIL: Testcase $HOME is not set, os=='nt'
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 154, in test_get_home_dir_5
    nt.assert_equal(home_dir, abspath(join(test_file_path, "home_test_dir")))
AssertionError: 'C:\\' !=
'/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'
>> raise self.failureExc...

Hi Fernando,

I have marked all tests but test_get_home_dir_3 as skip when not 
windows. This test should not fail with a 'undefined $HOME, IPython can 
not proceed.' because this function sets HOME to a known path.
I have tested on ubuntu 8.10 and it works for me now.

/Jörgen

Fernando Perez skrev:
> Hi Jorgen,
> 
> On Sun, Feb 1, 2009 at 7:18 AM, Jörgen Stenarson
> <jorgen.stenarson@bostream.nu> wrote:
>> Hi Fernando,
>>
>> I think I have fixed the failures. I have tested on a Ubuntu machine
>> successfully.
> 
> Mmh, I'm still getting errors and failures:
> 
> ======================================================================
> ERROR: Testcase $HOME is set, then use its value as home directory.
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
> line 182, in runTest
>     self.test(*self.arg)
>   File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
> line 132, in test_get_home_dir_3
>     home_dir = genutils.get_home_dir()
>   File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
> line 949, in get_home_dir
>     raise HomeDirError,'undefined $HOME, IPython can not proceed.'
> HomeDirError: undefined $HOME, IPython can not proceed.
> 
> ======================================================================
> ERROR: Testcase $HOME is not set, os=='nt'
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
> line 182, in runTest
>     self.test(*self.arg)
>   File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
> line 168, in test_get_home_dir_6
>     home_dir = genutils.get_home_dir()
>   File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
> line 957, in get_home_dir
>     raise HomeDirError
> HomeDirError
> 
> ======================================================================
> FAIL: Testcase for py2exe logic, un-compressed lib
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
> line 182, in runTest
>     self.test(*self.arg)
>   File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
> line 114, in test_get_home_dir_1
>     nt.assert_equal(home_dir, abspath(join(test_file_path, "home_test_dir")))
> AssertionError: '/home/fperez' !=
> '/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'
>>>  raise self.failureException, \
>           (None or '%r != %r' % ('/home/fperez',
> '/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'))
> 
> 
> ======================================================================
> FAIL: Testcase $HOME is not set, os=='nt'
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
> line 182, in runTest
>     self.test(*self.arg)
>   File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
> line 154, in test_get_home_dir_5
>     nt.assert_equal(home_dir, abspath(join(test_file_path, "home_test_dir")))
> AssertionError: 'C:\\' !=
> '/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'
>>>  raise self.failureException, \
>           (None or '%r != %r' % ('C:\\',
> '/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/home_test_dir'))
> 
> 
> ----------------------------------------------------------------------
> Ran 431 tests in 43.138s
> 
> FAILED (SKIP=6, errors=2, failures=2)
> 
> 
> Do you not get these?  I'm on ubuntu 8.10...
> 
> Cheers,
> 
> f

Revision history for this message

Fernando Perez (fdo.perez) wrote on 2009-02-11:

#

Hi Jorgen,

On Wed, Feb 11, 2009 at 9:39 AM, Jörgen Stenarson
<email address hidden> wrote:
> Hi Fernando,
>
> I have marked all tests but test_get_home_dir_3 as skip when not
> windows. This test should not fail with a 'undefined $HOME, IPython can
> not proceed.' because this function sets HOME to a known path.
> I have tested on ubuntu 8.10 and it works for me now.

Thanks for working on this. I'm getting (ubuntu 8.10, 32 bit):

....
IPython.tests.test_magic.test_rehashx ... ok

======================================================================
ERROR: Testcase $HOME is set, then use its value as home directory.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py",
line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_genutils.py",
line 133, in test_get_home_dir_3
    home_dir = genutils.get_home_dir()
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/genutils.py",
line 949, in get_home_dir
    raise HomeDirError,'undefined $HOME, IPython can not proceed.'
HomeDirError: undefined $HOME, IPython can not proceed.

----------------------------------------------------------------------
Ran 431 tests in 57.709s

FAILED (SKIP=9, errors=1)

I'm at a conference so I may not be able to help much right now, but
if I can I'll try to debug it tonight. If you have any ideas I can
test quickly any updates you make.

Once we sort this out, we'll merge yours.

Cheers,

f

Revision history for this message

Jörgen Stenarson (jorgen-stenarson) wrote on 2009-02-12:

#

Fernando Perez skrev:
>
> I'm at a conference so I may not be able to help much right now, but
> if I can I'll try to debug it tonight. If you have any ideas I can
> test quickly any updates you make.
> Once we sort this out, we'll merge yours.
>

I have no idea what the problem can be as it works for me in ubuntu.

some ideas for debugging:

Perhaps you can check if you get the same error when putting the
offending test in a separate test file, to see if there is some
interaction problem with the other tests in the file.

use the --pdb flag and check where HOME is defined and where it
disappears in the function.

I'll be away over the weekend coming back on sunday afternoon.

/Jörgen

Revision history for this message

Fernando Perez (fdo.perez) wrote on 2009-03-10:

#

Hi Jorgen,

"I think I have fixed the failures. I have tested on a Ubuntu machine successfully.
"

As I mentioned on-list, there were indeed some problems with the code, which I've now understood. Please have a look at the fixed code here:

lp:~fdo.perez/jorgen-py2exe-fix-dev

Note that this MAY now have problems on win32 with py2exe, because I shifted to using a temp directory for the tests, and that could conflict with some of your code. But it's important that the core of my fixes stay, because the code as it was did have a bug, and furthermore, it would not run correctly if the user had no write access to the test dir (normal in a system-wide install).

So please check if my changes broke something on win32, but without undoing what they fix :)

I also cleaned things up so that some of the key paths used are computed only once at the top, and kept as (uppercased) constants. They were hardcoded as join(....,'somestring') in many places, which isn't good. Explicitly handcoded strings that are repeated multiple times are a problem waiting to happen.

I think we're closing in on this one...

lp:~jorgen-stenarson/ipython/ipython-py2exe-fix updated on 2009-03-11

1167. By Jörgen Stenarson on 2009-03-11: Fixed errors in testcases specific to py2exe after Fernando's patch

Revision history for this message

Jörgen Stenarson (jorgen-stenarson) wrote on 2009-03-11:

#

Fernando Perez skrev:
> Hi Jorgen,
>
> "I think I have fixed the failures. I have tested on a Ubuntu machine successfully.
> "
>
> As I mentioned on-list, there were indeed some problems with the code, which I've now understood. Please have a look at the fixed code here:
>
> lp:~fdo.perez/jorgen-py2exe-fix-dev
>
> Note that this MAY now have problems on win32 with py2exe, because I shifted to using a temp directory for the tests, and that could conflict with some of your code. But it's important that the core of my fixes stay, because the code as it was did have a bug, and furthermore, it would not run correctly if the user had no write access to the test dir (normal in a system-wide install).
>
> So please check if my changes broke something on win32, but without undoing what they fix :)
>
> I also cleaned things up so that some of the key paths used are computed only once at the top, and kept as (uppercased) constants. They were hardcoded as join(....,'somestring') in many places, which isn't good. Explicitly handcoded strings that are repeated multiple times are a problem waiting to happen.
>
> I think we're closing in on this one...
Hi,

I have fixed some problems with the py2exe tests hopefully without
messing up your changes:) So now it works fine on win32.

However there is one change you made in genutils.get_home_dir() that
maybe should be changed. You reraise a ValueError on line 949 but this
error is not caught by the enclosing except. Is this just a forgotten
debugging change or is there something else behind it?

/Jörgen

Revision history for this message

Fernando Perez (fdo.perez) wrote on 2009-03-11:

#

On Wed, Mar 11, 2009 at 10:51 AM, Jörgen Stenarson
<email address hidden> wrote:

> I have fixed some problems with the py2exe tests hopefully without
> messing up your changes:) So now it works fine on win32.

Great, thanks! I have a visitor all day today, I'll look into it tomorrow.

>
> However there is one change you made in genutils.get_home_dir() that
> maybe should be changed. You reraise a ValueError on line 949 but this
> error is not caught by the enclosing except. Is this just a forgotten
> debugging change or is there something else behind it?

Sorry, forgotten debug change. I thought I'd pushed the fix, but I
guess I only committed it locally. Please have a look at my branch
now (I just pushed) and merge anything that might be there (perhaps
nothing).

Once this is good, we can merge your branch into trunk and close it
(as well as my dev copy of yours).

Thanks!

f

lp:~jorgen-stenarson/ipython/ipython-py2exe-fix updated on 2009-03-16

1168. By Jörgen Stenarson on 2009-03-16: Merged changes on trunk
1169. By Jörgen Stenarson on 2009-03-16: Removed some debug code in get_home_dir

IPython

Merge lp:~jorgen-stenarson/ipython/ipython-py2exe-fix into lp:ipython/0.11

Commit message

Description of the change

Subscribers