failing tests on windows

12 views
Skip to first unread message

g

unread,
Feb 15, 2012, 1:43:00 PM2/15/12
to pyjaco - The Python to Javascript compiler
This commit appears to be the culprit making tests fail on Windows:
https://github.com/chrivers/pyjaco/commit/3a115cb8e9a24bbab199180b23b3765fb4b16747

Also, testtools/env_tests.py was left out of this update. It still
uses os.system() so passed instead of failing.

Below is command window output demonstrating the issue for one test:

COMMAND:
"C:\Python27\python.exe" run_tests.py test_compile_js

Line:
self.assertEqual(0, subprocess.call([cmd], shell = True))

Result:
C:\path\to\pyjaco>"C:\Pyth
on27\python.exe" run_tests.py test_compile_js
Looking for "js" and "py-
builtins" [2]: .. [OK]

tests/test_compile_js.py [1]: '"C:\Python27\python.exe \"tests
\test_compile_js.p
y\" > \"tests\test_compile_js.py.out\" 2> \"tests
\test_compile_js.py.err\""' is
not recognized as an internal or external command,
operable program or batch file.
[FAIL]
Ran 2 tests in 0.084s

FAILED (failures=1)

Line:
self.assertEqual(0, subprocess.call([cmd], shell = False))

Result:
C:\path\to\pyjaco>"C:\Pyth
on27\python.exe" run_tests.py test_compile_js
Looking for "js" and "py-
builtins" [2]: .. [OK]

tests/test_compile_js.py
[1]: [Error]

Ran 2 tests in 0.067s

FAILED (errors=1)

errors:
(use -x to skip this part)

* tests/test_compile_js.py [1]: *
Traceback (most recent call last):
File "C:\path\to\pyjaco\
testtools\util.py", line 79, in runTest
self.assertEqual(0, subprocess.call([cmd], shell = False))
File "C:\Python27\lib\subprocess.py", line 493, in call
return Popen(*popenargs, **kwargs).wait()
File "C:\Python27\lib\subprocess.py", line 679, in __init__
errread, errwrite)
File "C:\Python27\lib\subprocess.py", line 893, in _execute_child
startupinfo)
WindowsError: [Error 2] The system cannot find the file specified

Line:
self.assertEqual(0, os.system(cmd))

Result:
C:\path\to\pyjaco>"C:\Pyth
on27\python.exe" run_tests.py test_compile_js
Looking for "js" and "py-
builtins" [2]: .. [OK]

tests/test_compile_js.py
[1]: . [OK]

Ran 2 tests in 0.112s

OK

g

unread,
Feb 15, 2012, 8:03:24 PM2/15/12
to pyjaco - The Python to Javascript compiler
So I made a minor tweak to the test running code and now the tests
pass for me on Windows. Revision 498aa55d7a7a of my repo at
https://bitbucket.org/pendletongp/pyjaco/

I set up an Ubuntu virtual box to make sure that the tests still
passed on Linux. Seemed to be okay. Would anyone mind confirming? A
few failed, but they failed in the official repo too. It just seems
like wrapping the cmd argument to subprocess.call in a list was
causing Windows to wrap the entire command in quotes causing
everything to fail.

I also made it so that the environment test used the same call code as
the rest of the tests. I think it might be a good idea to add a test
to env_test that checks to make sure the javascript load command is
available. Ubuntu told me to install nodejs to get the js command...
but node doesn't provide the "load" command like the d8 shell does.

Samuel Ytterbrink

unread,
Feb 17, 2012, 1:55:14 AM2/17/12
to pyj...@googlegroups.com
I have now read and confirmed this patch, up to rev f45da41947ae!

I dont remember about the [] more then that it was importent in some
other code. Its to bad that the comment of the revision you change it
dont state why, only that you remove them.

Could be good fore future reference if we find that it breaks, maybe on mac?

how ever i remember that on the other place we used the brackets we
had a check for the operating system.

I'll merge this soon, if i dont hear anything else from chrivers.

at least 2+ for good coding style!

2012/2/16 g <wgor...@gmail.com>:

> --
> You are subscribed to the Google Group pyj...@googlegroups.com
> To unsubscribe from this group, send email to
> pyjaco+un...@googlegroups.com

--
//Samuel Ytterbrink

g

unread,
Feb 17, 2012, 9:31:45 AM2/17/12
to pyjaco - The Python to Javascript compiler
On Feb 17, 1:55 am, Samuel Ytterbrink <nepp...@gmail.com> wrote:
> I have now read and confirmed this patch, up to rev f45da41947ae!
>

=) Thanks.

> I dont remember about the [] more then that it was importent in some
> other code. Its to bad that the comment of the revision you change it
> dont state why, only that you remove them.
>
> Could be good fore future reference if we find that it breaks, maybe on mac?
>

Whoops... you are definitely right. Git really needs a safe way to
modify commit comments.

> how ever i remember that on the other place we used the brackets we
> had a check for the operating system.
>

I could update it to use a Caller class that runs once on init and
sets the function to use. Could simply check to see if wrapping the
command in a list works, if not, fall back to using no list wrapper.
Could call something like "python --version" so we know the dependency
is met.

> I'll merge this soon, if i dont hear anything else from chrivers.
>
> at least 2+ for good coding style!
>

I will work on my commit messages. Sorry about that.

Samuel Ytterbrink

unread,
Feb 17, 2012, 10:41:53 AM2/17/12
to pyj...@googlegroups.com
2012/2/17 g <wgor...@gmail.com>:

> On Feb 17, 1:55 am, Samuel Ytterbrink <nepp...@gmail.com> wrote:
>> I have now read and confirmed this patch, up to rev f45da41947ae!
>>
>
> =) Thanks.
>
>> I dont remember about the [] more then that it was importent in some
>> other code. Its to bad that the comment of the revision you change it
>> dont state why, only that you remove them.
>>
>> Could be good fore future reference if we find that it breaks, maybe on mac?
>>
>
> Whoops... you are definitely right.  Git really needs a safe way to
> modify commit comments.

I think the problem is that it is _safe_. if none else have made a
pull you could always use rebase ^^.

>
>> how ever i remember that on the other place we used the brackets we
>> had a check for the operating system.
>>
>
> I could update it to use a Caller class that runs once on init and
> sets the function to use.  Could simply check to see if wrapping the
> command in a list works, if not, fall back to using no list wrapper.
> Could call something like "python --version" so we know the dependency
> is met.
>
>> I'll merge this soon, if i dont hear anything else from chrivers.
>>
>> at least 2+ for good coding style!
>>
>
> I will work on my commit messages.  Sorry about that.
>

I woulden't do a better one my self, it's hard to do good commit
messages on the fly.

Dusty Phillips

unread,
Feb 17, 2012, 11:05:18 AM2/17/12
to pyj...@googlegroups.com
On 17/02/12 08:41 AM, Samuel Ytterbrink wrote:
> 2012/2/17 g<wgor...@gmail.com>:
>> On Feb 17, 1:55 am, Samuel Ytterbrink<nepp...@gmail.com> wrote:
>>> I have now read and confirmed this patch, up to rev f45da41947ae!
>>>
>>
>> =) Thanks.
>>
>>> I dont remember about the [] more then that it was importent in some
>>> other code. Its to bad that the comment of the revision you change it
>>> dont state why, only that you remove them.
>>>
>>> Could be good fore future reference if we find that it breaks, maybe on mac?
>>>
>>
>> Whoops... you are definitely right. Git really needs a safe way to
>> modify commit comments.
>
> I think the problem is that it is _safe_. if none else have made a
> pull you could always use rebase ^^.

I agree. When you are constructing a repo or branch for review by other
developers, but that is not being pulled into the mainline repo, it is
not only acceptable, but desirable to modify git history to create the
most obvious explanation of development. You can use git commit --amend
to modify the most recent commit, and git rebase to modify earlier
revisions. The goal is to create a series of commits that is
understandable and obvious, with each commit having it's own obviously
related set of changes.

Cheers,

Dusty

g

unread,
Feb 18, 2012, 12:19:11 AM2/18/12
to pyjaco - The Python to Javascript compiler
Okay. So I tried to figure out what was going on to cause the issue
but I could not. I modified the commit msg with an example of the
problem since I couldn't explain why it occurred.
https://bitbucket.org/pendletongp/pyjaco/changeset/1c36e39865e8

Is this what you are looking for?

Samuel Ytterbrink

unread,
Feb 18, 2012, 4:14:02 AM2/18/12
to pyj...@googlegroups.com
actually i only needed this part:
"do not wrap the input command to subprocess.call with a list
because this causes tests to fail on Windows."

or shorter
"do not wrap the input command to subprocess.call with a list, faild
on Windows."

I may have missed the last bart before, was it there?

Anny ways, thx!

2012/2/18 g <wgor...@gmail.com>:

g

unread,
Feb 18, 2012, 9:37:35 AM2/18/12
to pyjaco - The Python to Javascript compiler
>
> I may have missed the last bart before, was it there?
>

I mentioned that it caused tests to fail on Windows. I just added
command line output demonstrating the issue because I wasn't really
sure what caused it. The problem didn't seem to show up with Python
2.6, just Python 2.7. I figured this would be relevant if someone
changed it in the future.

Here was the msg:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
do not wrap the input command to subprocess.call with a list
because this causes tests to fail on Windows. Below shows this on
Python
2.7.2

System Information:
>>> sys.getwindowsversion()
(6, 1, 7601, 2, 'Service Pack 1')

Python 2.7.2
>>> cmd = "echo | js > %s" % os.path.join(tempfile.gettempdir(), tempfile.gettempprefix())
>>> cmd
'echo | js > c:\\users\\gordon\\appdata\\local\\temp\\tmp'
>>> os.system(cmd)
0
>>> subprocess.call([cmd], shell = True)
The filename, directory name, or volume label syntax is incorrect.
1
>>> subprocess.call(cmd, shell = True)
0

Python 2.6.6
>>> cmd = "echo | js > %s" % os.path.join(tempfile.gettempdir(), tempfile.gettempprefix())
>>> cmd
'echo | js > c:\\users\\gordon\\appdata\\local\\temp\\tmp'
>>> os.system(cmd)
0
>>> subprocess.call([cmd], shell = True)
0
>>> subprocess.call(cmd, shell = True)
0
>>>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Samuel Ytterbrink

unread,
Feb 18, 2012, 3:41:05 PM2/18/12
to pyj...@googlegroups.com
ok thx, then i would say it was enough to say that the bug was
intredused on 2.7 on windows or something.

Sorry that i didden't se the long commit message before, my bad.

If you are ok with the current commit message then i will try to
merge it in to the 'main' repo.

2012/2/18 g <wgor...@gmail.com>:

g

unread,
Feb 18, 2012, 4:28:53 PM2/18/12
to pyjaco - The Python to Javascript compiler


On Feb 18, 3:41 pm, Samuel Ytterbrink <nepp...@gmail.com> wrote:
> ok thx, then i would say it was enough to say that the bug was
> intredused on 2.7 on windows or something.
>
> Sorry that i didden't se the long commit message before, my bad.
>
> If you are ok with the current commit message then i will  try to
> merge it in to the 'main' repo.
>

Okay. Simplified it so the revision numbers changed again. I think
you want to pull from revision 6f1d1eeb40c2 now.

Samuel Ytterbrink

unread,
Feb 22, 2012, 3:53:29 PM2/22/12
to pyj...@googlegroups.com
changes are in!

I dont know how to ad the tag. Can someone shed some light?

2012/2/18 g <wgor...@gmail.com>:

g

unread,
Feb 22, 2012, 3:58:31 PM2/22/12
to pyjaco - The Python to Javascript compiler
> I dont know how to ad the tag. Can someone shed some light?

I think something along these lines:

git tag -a -m "initial version tag" 1.3
git push --tags


That is just from memory though. You need the -a flag to create a tag
that will work. Then you have to push with --tags. You may just need
to do the git push --tags though. Because if you pulled from my repo
the tag should already be there I think?

Samuel Ytterbrink

unread,
Feb 22, 2012, 4:02:50 PM2/22/12
to pyj...@googlegroups.com
thx tag is up!

it was only

git push --tags

2012/2/22 g <wgor...@gmail.com>:

g

unread,
Feb 22, 2012, 4:05:20 PM2/22/12
to pyjaco - The Python to Javascript compiler
> changes are in!

hurrah!

g

unread,
Feb 22, 2012, 4:12:00 PM2/22/12
to pyjaco - The Python to Javascript compiler
On Feb 22, 4:02 pm, Samuel Ytterbrink <nepp...@gmail.com> wrote:
> thx tag is up!
>
> it was only
>
> git push --tags
>
> 2012/2/22 g <wgordo...@gmail.com>:

Cool cool... just pulled and version info is working =). Output is:
1.3.2+6f1d1ee
Reply all
Reply to author
Forward
0 new messages