-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
bpo-26053: Correct assertion in pdb test #25139
Conversation
This passed on a mac (where master currently fails). How do we check why the CI didn't find the issue? |
The previous PR had been open for a long time. Could it be that the tests used to pass, and now when it was merged they were not executed again? |
self.assertIn("Restarting main.py with arguments:\nd e f", output) | ||
res = '\n'.join([x.strip() for x in stdout.splitlines()]) | ||
self.assertRegex(res, "Restarting .* with arguments:\na b c") | ||
self.assertRegex(res, "Restarting .* with arguments:\nd e f") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use script? Maybe you need os.path.basename(), I don't know pdb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is script? The important part of the test is the "a b c" and "d e f"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filename comes from Bdb.canonic:
```
def canonic(self, filename):
"""Return canonical form of filename.
For real filenames, the canonical form is a case-normalized (on
case insensitive filesystems) absolute path. 'Filenames' with
angle brackets, such as "<stdin>", generated in interactive
mode, are returned unchanged.
"""
if filename == "<" + filename[1:-1] + ">":
return filename
canonic = self.fncache.get(filename)
if not canonic:
canonic = os.path.abspath(filename)
canonic = os.path.normcase(canonic)
self.fncache[filename] = canonic
return canonic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to use canonic(). It passes on the mac.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see what you mean - no, script is the code (contents of the script). The name is main.py, hard-coded in run_pdb_script()
I think this is the reason the CI missed it - I can see that new open PRs are blocked now with this error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if the tests pass ;-)
I think they fail on windows. |
c:\ vs C:\ |
688abc5
to
4d99340
Compare
I reverted it back - it's not simple to get to the pdb filename from here (the pdb instance is destroyed in run_pdb_script and we only have the stdin/stdout in the test function). And, as I said, the filename is not the point of this test anyway, the point is to see that the correct args are picked up. |
(cherry picked from commit bd4ab8e)
* bpo-26053: Fix args echoed by pdb run command (pythonGH-22033) (cherry picked from commit 652bfde) * bpo-26053: Fix test_pdb.test_issue26053() (pythonGH-25139) (cherry picked from commit bd4ab8e) (cherry picked from commit 7ad56e2) Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
https://bugs.python.org/issue26053