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

A lot of testerrors #1148

Closed
jstenar opened this issue Dec 13, 2011 · 5 comments
Closed

A lot of testerrors #1148

jstenar opened this issue Dec 13, 2011 · 5 comments

Comments

@jstenar
Copy link
Member

jstenar commented Dec 13, 2011

I get 3 failures in IPython.testing, 16 errors and 10 failures in IPython.core on master 1184eb4, on windows python2.6.

All but one go away if I add the strict keyword argument to arg_split in _process_win32.py

diff --git a/IPython/utils/_process_win32.py b/IPython/utils/_process_win32.py
index 9de9edf..77a30a1 100644
--- a/IPython/utils/_process_win32.py
+++ b/IPython/utils/_process_win32.py
@@ -159,7 +159,7 @@ try:
     LocalFree.res_type = HLOCAL
     LocalFree.arg_types = [HLOCAL]

-    def arg_split(commandline, posix=False):
+    def arg_split(commandline, posix=False, strict=False):
     """Split a command line's arguments in a shell-like manner.

     This is a special version for windows that use a ctypes call to CommandLineToArgvW

After that only one error is remaining, and that one is included below. Is this related to arg_split differences on windows or is it present in Linux as well?

ERROR: test shlex issues with timeit (#1109)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\python26\lib\site-packages\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "c:\python\external\ipython\IPython\core\tests\test_magic.py", line 350, in test_timeit_shlex
    _ip.magic('timeit -n1 "this is a bug".count(" ")')
  File "c:\python\external\ipython\IPython\core\interactiveshell.py", line 1995, in magic
    result = fn(magic_args)
  File "c:\python\external\ipython\IPython\core\magic.py", line 1893, in magic_timeit
    code = compile(src, "<magic-timeit>", "exec")
  File "<magic-timeit>", line 6
    this is a bug.count( )
        ^
SyntaxError: invalid syntax
@minrk
Copy link
Member

minrk commented Dec 13, 2011

Arg, yes. Since that was a test specifically for a bug fixed by the strict=False behavior, it just goes to show that we really shouldn't be pretending that magic args are like native command-line args. Since behavior should not have changed at all, I would just mark the new test as a known failure on Windows.

@takluyver
Copy link
Member

Oops, I guess we overlooked changing the function signature for that implementation of arg_split.

I'm not seeing any failures on Linux, so I guess that bug is in the Windows version of arg_split. Has that test passed at any time since your PR #1064? There's probably no good tools to handle the mixture of command-line syntax and Python code we're trying to use here.

@minrk
Copy link
Member

minrk commented Dec 13, 2011

@takluyver - it's a new test, so it wasn't around when 1064 went in. Yes, it's definitely a 'bug' in arg_split, but only insofar as we use arg_split for things other than passing command-line args to subprocesses, which we probably shouldn't.

I think the answer is that we should force using the _process_common arg_split in magics. @jstenar - is there anything that your new arg_split does that is actually necessary in magics? The uglier alternative is that we can fall _process_common.arg_split() from win32.argsplit if strict=False, but I would rather not do that unless it's strictly necessary.

@jstenar
Copy link
Member Author

jstenar commented Dec 13, 2011

Min RK skrev 2011-12-13 19:52:

@takluyver - it's a new test, so it wasn't around when 1064 went in. Yes, it's definitely a 'bug' in arg_split, but only insofar as we use arg_split for things other than passing command-line args to subprocesses, which we probably shouldn't.

I think the answer is that we should force using the _process_common arg_split in magics. @jstenar - is there anything that your new arg_split does that is actually necessary in magics? The uglier alternative is that we can fall _process_common.arg_split() from win32.argsplit if strict=False, but I would rather not do that unless it's strictly necessary.

Yes it strips out enclosing quotation marks and I believe there are
differences with how backspace is handled.

In [1]: import IPython.utils._process_win32 as pw

In [2]: import IPython.utils._process_common as pc

In [3]: pw.arg_split(u'%run "kalle.py"')
Out[3]: [u'%run', u'kalle.py']

In [4]: pc.arg_split(u'%run "kalle.py"')
Out[4]: [u'%run', u'"kalle.py"']

@minrk
Copy link
Member

minrk commented Dec 13, 2011

Thanks, I saw your original PR/bug report, and now I better understand the situation. I started off with the assumption that this was old code, not new. See PR #1149 for a band-aid solution.

@minrk minrk closed this as completed in 26662f4 Dec 13, 2011
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
if strict=False, falls back on shlex-based split from _process_common

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

No branches or pull requests

3 participants