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
BUG: fix broken tests for versioned_branches feature #533
BUG: fix broken tests for versioned_branches feature #533
Conversation
c213c31
to
59ba7b8
Compare
yield ast_to_offset(node), _fix_py3_block | ||
else: | ||
# don't try to fix, e.g., >= (3, 6) if there's no else clause | ||
yield ast_to_offset(node), lambda i, tokens: None |
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.
we shouldn't put noop callbacks into the callback machinery -- I think the original logic was fine here (?)
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.
actually the result with the original logic for that one broken test is
import sys
if True:
- if sys.version_info >= (3, 6):
- pass
+ pass
so that sounds about right to me, I guess this test was misplaced and shouldn't have been in the "noop" zone at all ?
I'll push to revert this.
'import sys' | ||
'if True' | ||
'import sys\n' | ||
'if True\n' |
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 yeah this'll need a :
too
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.
good catch
'if sys.version_info >= (3, 6):\n' | ||
' pass' | ||
) | ||
expected = 'import sys\n' |
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 expect import sys\npass\n
-- and my checkout of pyupgrade
correctly makes that happen (?)
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.
you're right, sorry, it's getting late in 🇫🇷
c3d71b9
to
1e9a2fd
Compare
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.
wait don't merge I can add an id to the test |
1e9a2fd
to
9adb0c6
Compare
Followup to #530, thanks to a comment from @graingert
Actually the bug now fails, revealing a bug in #530 ? I'll try to fix this promptly