Increase test coverage #46
Conversation
asttokens/util.py
Outdated
| @@ -167,8 +162,6 @@ def visit_tree(node, previsit, postvisit): | |||
|
|
|||
| For the initial node, ``par_value`` is None. Either ``previsit`` and ``postvisit`` may be None. | |||
There was a problem hiding this comment.
It seems "previsit may be None" comment is no longer accurate.
| @@ -99,10 +99,7 @@ def _visit_after_children(self, node, parent_token, token): | |||
| node.last_token = nlast | |||
|
|
|||
| def _find_last_in_line(self, start_token): | |||
| try: | |||
| newline = self._code.find_token(start_token, token.NEWLINE) | |||
| except IndexError: | |||
There was a problem hiding this comment.
Mmmm, I have a feeling removing this except can easily cause an error. Why do you think it's better to remove?
There was a problem hiding this comment.
I'm removing code that never runs. find_token doesn't advance past the end because it has a check and not token.ISEOF(t.type). We should maintain that guarantee and also trust it.
| maybe_comma = self._code.next_token(last_token) | ||
| if util.match_token(maybe_comma, token.OP, ','): | ||
| last_token = maybe_comma | ||
| except IndexError: |
There was a problem hiding this comment.
next_token can't raise an error here because there's always an endmarker in tokens, which I've just added a check for. If last_token is already the endmarker then that's a problem which should be fixed. I think our extensive tests, particularly for tuples, are sufficient.
| assert filename == atok.filename | ||
|
|
||
|
|
||
| def test_doesnt_have_location(): |
There was a problem hiding this comment.
Could you add a comment what this tests, please? The test name isn't self-explanatory.
|
Hey, you approved the changes, but I think you forgot you need to merge. Also please release afterwards. There's an actual bug that's been fixed here (the use of |
|
You got it. |
No description provided.