Closed schodge closed 10 years ago
schodge
commented
10 years ago OK, the first git error is because git checkout -b ter is being run at a time when ter already exists - from looking at the repo when pdb halts nose:
c:\github\pep8radius\tests\temp (master)
λ git branch
* master
ter
c:\github\pep8radius\tests\temp (master)
λ git checkout -b ter
fatal: A branch named 'ter' already exists.ERROR: test_earlier_revision (test_pep8radius.TestRadiusHg)is failing because there's no earlier revision:
(Pdb) p popenargs
(['hg', 'debugancestor', False, u'000000000000'],)c:\github\pep8radius\tests\temp (master)
λ hg log
c:\github\pep8radius\tests\temp (master)
λ hg id
000000000000+ (ter) tip
hayd
commented
10 years ago This sucks.
schodge
commented
10 years ago We wouldn't get paid the big bucks for this if it were easy.
hayd
commented
10 years ago :D. Hmmm many of these errors I've no idea about... maybe just to confirm you're on latest autopep8 and docformatter.
I guess the branch still existing means the clean up (which deletes the .hg directory each time TestRadiusHg.__init__ is run) isn't working. Eurgh!
Can you try manually deleting the temp directory completely before running tests?
schodge
commented
10 years ago Autopep8 and docformatter are current to what's on PyPI.
Deleted everything in temp, same error.
hayd
commented
10 years ago drat...
hayd
commented
10 years ago ...wait when you say deleted everything in temp, did you try deleting temp? Those are dot files so may be hidden (.hg and .git directories). :s
solved the skipping bzr thing (the skiptest was in check rather than setUp).
schodge
commented
10 years ago OK, deleting temp itself fixes those two errors about the branch. Others still present.
hayd
commented
10 years ago Can you pdb into test_earlier_revision (test_pep8radius.TestRadiusHg) and say what's rev1 and rev2?
Aaargh Windows.
It's weird cos the others reads as if it's tracking all the files in the temp directory when I would expect it (slash, on Windows/Mac) it only tracks those which we've explicitly added. At least that's part of the issue. :tears:
schodge
commented
10 years ago http://stackoverflow.com/questions/4950637/setting-breakpoints-with-nosetests-pdb-option
Gonna tattoo this onto the back of my hand. And perhaps one day I'll find out why replacing pdb with ipdb creates an infinite loop.
schodge
commented
10 years ago Here's an edited log as I step through the code. Note that hg log and hg branches return nothing. Also, out is empty here, will restep through the code and try to figure out why, have seen this occur frequently in the past, usually seemed to be the result of a syntax error to hg. (Note the parenthetical (ter) in the shell dump is an indication of the git repo).
> c:\github\pep8radius\pep8radius.py(56)shell_out()
-> try:
(Pdb) l
51 subprocess.CalledProcessError = CalledProcessError
52
53
54 def shell_out(cmd, stderr=STDOUT):
55 out = subprocess.check_output(cmd, stderr=stderr, universal_newlines=True)c:\github\pep8radius\tests\temp (master)
λ hg status
A a.py
? .git\COMMIT_EDITMSG
? .git\HEAD
? .git\config
? .git\description
? .git\hooks\applypatch-msg.sample
? .git\hooks\commit-msg.sample
? .git\hooks\post-update.sample
? .git\hooks\pre-applypatch.sample
? .git\hooks\pre-commit.sample
? .git\hooks\pre-push.sample
? .git\hooks\pre-rebase.sample
? .git\hooks\prepare-commit-msg.sample
? .git\hooks\update.sample
? .git\index
? .git\info\exclude
? .git\logs\HEAD
? .git\logs\refs\heads\master
? .git\logs\refs\heads\ter
? .git\objects\16\629302176003ffd8f1ae258aa092b53f3b8849
? .git\objects\16\eb0a424d9053e80e75dce4a1e5cd4d4bee2a69
? .git\objects\1d\0a59ee6266492199862fd8030e6388151cde1f
? .git\objects\21\814fac2cb90959578b901c4e40183b8f74d347
? .git\objects\22\687adaa1605c7c8503ca445ef1af55c3bf22e4
? .git\objects\23\ea2e57341a29a6766cf3d03b42a36c704f899d
? .git\objects\24\cbc2f990d7b54b8630b4c903e96e4f00ccd0b7
? .git\objects\3b\92032fdcc28ba1dab3031e8e14c55f4f2649cc
? .git\objects\47\878b738b245e4678b2cb89f818595530cb48b4
? .git\objects\51\b30f933ab6b3f3885cef0dfdb4fb3d95d787c4
? .git\objects\59\f6077ab5209a83045a25c9975d60e46e26366d
? .git\objects\6c\171d4cc8bd01e952f97fc1f461a99ab4de9963
? .git\objects\74\b6831cc05d24e2b8f0b1d5a1f19a892f501f1d
? .git\objects\89\49d80da5f0e88b4d413f08b84644b6ee7a9151
? .git\objects\98\9e7e24c2c2296d06f8afb8bc1fa29a3192266b
? .git\objects\c4\d44af8dc3653f7c16ad7b07679e265cf7b587a
? .git\objects\c5\eecb694efc6c9bb0d472401e651041e0085689
? .git\refs\heads\master
? .git\refs\heads\ter
? AAA.py
? BBB.py
? CCC.py
? temp.py
c:\github\pep8radius\tests\temp (ter)
λ hg log
c:\github\pep8radius\tests\temp (ter)
λ hg branch
default
c:\github\pep8radius\tests\temp (ter)
λ hg branches
c:\github\pep8radius\tests\temp (ter)
λ hg summary
parent: -1:000000000000 tip (empty repository)
branch: default
commit: 1 added, 41 unknown (new branch head)
update: (current)
c:\github\pep8radius\tests\temp (ter)
λ hg summary
parent: -1:000000000000 tip (empty repository)
branch: default
commit: 2 added, 40 unknown (new branch head)
update: (current)
c:\github\pep8radius\tests\temp (ter)
λ hg branches
c:\github\pep8radius\tests\temp (ter)
λ hg branch
defaultOh, dear me, what a very funny thing to be potentially be the cause of so much trouble. Please remember, violence is not a solution, even to dimwitted Windows peeps who didn't bother to actually set up Hg.
> c:\anaconda\lib\subprocess.py(568)check_output()
-> retcode = process.poll()
(Pdb) p output
'abort: no username supplied\n(use "hg config --edit" to set your username)\n'
(Pdb) l
563 """
564 if 'stdout' in kwargs:
565 raise ValueError('stdout argument not allowed, it will be overridden.')
566 process = Popen(stdout=PIPE, *popenargs, **kwargs)
567 output, unused_err = process.communicate()
568 -> retcode = process.poll()
569 if retcode:
570 cmd = kwargs.get("args")
571 if cmd is None:
572 cmd = popenargs[0]
573 raise CalledProcessError(retcode, cmd, output=output)
(Pdb)(Pdb) u
> c:\github\pep8radius\pep8radius.py(55)shell_out()
-> out = subprocess.check_output(cmd, stderr=stderr, universal_newlines=True)
(Pdb) l
50 # keyword not being available (in 2.6)
51 subprocess.CalledProcessError = CalledProcessError
52
53
54 def shell_out(cmd, stderr=STDOUT):
55 -> out = subprocess.check_output(cmd, stderr=stderr, universal_newlines=True)Step through it a bit.
> c:\anaconda\lib\subprocess.py(573)check_output()
-> raise CalledProcessError(retcode, cmd, output=output)
(Pdb) l
568 retcode = process.poll()
569 if retcode:
570 cmd = kwargs.get("args")
571 if cmd is None:
572 cmd = popenargs[0]
573 -> raise CalledProcessError(retcode, cmd, output=output)
574 return output
575
576
577 def list2cmdline(seq):
578 """
(Pdb) p retcode
255
(Pdb) p cmd
['hg', 'commit', '-m', 'initial_commit']
(Pdb) p output
'abort: no username supplied\n(use "hg config --edit" to set your username)\n'I think there winds up being a return False:
> c:\github\pep8radius\tests\test_pep8radius.py(374)successfully_commit_files()
-> return False
(Pdb) l
369 try:
370 shell_out(["hg", "add"] + file_names)
371 shell_out(["hg", "commit", "-m", commit])
372 return RadiusHg.current_branch()
373 except (OSError, CalledProcessError):
374 -> return FalseAnd so the actual hg error doesn't git captured by out = subprocess.check_output(cmd, stderr=stderr, universal_newlines=True), instead it's empty. I think it'd be better to raise the error message up to the nosetests --verbose output.
schodge
commented
10 years ago Bumping. As an idea, both to fix this and for ease of maintenance, maybe we have one layer of abstraction too many for a project of this size? Maybe just propagating the error another level up on that last code sample makes sense. I defer to your judgment on this - my attempts at architecture are wanting. (There's a great comic I can't find on this - programmer's vision vs. implementation, with a house).
Also, getting a new problem on TestRadiusGit - looks like Note to self: don't use random_junk_sha is acting up again, but it just sorta hangs / infinite loop / waits forever. Do you know a way to forcibly drop into pdb on a ctrl-c (or something) so I don't have to experiment with set_trace() until I can find the sticking point?set_trace(), forget about it, and then not use -s on nosetests.
hayd
commented
10 years ago Never quite got how nosetests plays with capturing stdout, the thing is we already capture it in the test (so we can integration test!). Maybe it would be easier just not to integration test (but the code is already there... except for Windows).
I still don't get how hg can pass on mac/linux, but fail on windows. I think it's an hg issue (behaviour differs on platforms)...
schodge
commented
10 years ago Check the long post before my last one. Hg problem was that user name wasn't set, but that error gets swallowed before it propogates to the output.
If you have pdb.set_trace() somewhere in your test code and nosetests executes it, if you forget the -s flag stdout is captured, and it appears as though the test has hung. That was my problem on the crossed out paragraph.
EDIT: Hg works once the user name is set. I imagine you'd have the same trouble on OSX/Linux if you delete the user name from the config file (i.e., how a fresh install looks).
hayd
commented
10 years ago So the whole ter thing was a red herring? (was to do with your user config?)
nose -s thing is often broken... I is often fixed by just using pytest!
schodge
commented
10 years ago It's a red herring, but take a look at the long post I have about how the useful error message from Hg gets wiped out before it percolates all the way up. I think we need to make sure the error messages get out and don't get wiped clean and become an unexplained failure.
369 try:
370 shell_out(["hg", "add"] + file_names)
371 shell_out(["hg", "commit", "-m", commit])
372 return RadiusHg.current_branch()
373 except (OSError, CalledProcessError):
374 -> return FalseInstead of just returning false, it make sense to also pass back the errors as return (False, OSError, CalledProcessError).
hayd
commented
10 years ago This are/should be reraised in main, at least the CalledProcessError ?
schodge
commented
10 years ago Delete your Hg username for your Hg config and run nosetests and see what you get.
What should happen is what happens in the first post of this thread - you get back a bunch of errors that make it look like the comparison to the test is failing; if you dig a little deeper, it looks like Hg is returning an empty string for some reason, but if you finally (and belatedly on my part, sorry) go all the way to the bottom of the stack trace you find Hg is returning an error about the username, but it looks like the return False clobbers that error message, and so nosetests output makes it look like something else is the problem.
Anyway, I'd recommend trying it on your build (the delete Hg username) and see if that error makes it out of nosetests or if something else percolates up.
hayd
commented
10 years ago I'm ok with the tests failing like that if not properly configured. More important is how the script itself handles lack of username etc.
On 14 August 2014 12:13, Shayne Hodge notifications@github.com wrote:
Delete your Hg username for your Hg config and run nosetests and see what you get.
What should happen is what happens in the first post of this thread - you get back a bunch of errors that make it look like the comparison to the test is failing; if you dig a little deeper, it looks like Hg is returning an empty string for some reason, but if you finally (and belatedly on my part, sorry) go all the way to the bottom of the stack trace you find Hg is returning an error about the username, but it looks like the return False clobbers that error message, and so nosetests output makes it look like something else is the problem.
Anyway, I'd recommend trying it on your build (the delete Hg username) and see if that error makes it out of nosetests or if something else percolates up.
— Reply to this email directly or view it on GitHub https://github.com/hayd/pep8radius/issues/61#issuecomment-52229649.
Should the first test be skipping if no Bzr is installed?