Skip to content

Annotate functions in tests #4434

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 10, 2018
Merged

Annotate functions in tests #4434

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36a1637
annotate functions in tests
elazarg Jan 7, 2018
8c5be18
fix and duplicate tests as required
elazarg Jan 8, 2018
815a3f5
inplace addition for list; fix InvalidAttribute
elazarg Jan 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test-data/unit/check-classes.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ A().f = None # E: Cannot assign to a method
[case testReferToInvalidAttribute]

class A:
def __init__(self):
def __init__(self) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that omitting the return type was intentional here. It would be okay to have two variants of this method, one with an annotation for __init__ and another without.

self.x = object()
a = None # type: A
a.y
Expand Down
8 changes: 4 additions & 4 deletions test-data/unit/check-inference.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,7 @@ b[{}] = 1

[case testInferDictInitializedToEmptyAndUpdatedFromMethod]
map = {}
def add():
def add() -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear if the annotation was omitted intentionally. I'd suggest creating two variants of the test case.

map[1] = 2
[builtins fixtures/dict.pyi]
[out]
Expand Down Expand Up @@ -1959,7 +1959,7 @@ class C:

if bool():
f()
1 + '' # E: Unsupported left operand type for + ("int")
1 + '' # E: Unsupported operand types for + ("int" and "str")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer changing the test case so that we don't depend on int.__add__ instead (it's a little arbitrary to rely on it here). For example, use 1() instead of 1 + '' which doesn't require a fixture change.

[builtins fixtures/list.pyi]
[out]

Expand All @@ -1976,7 +1976,7 @@ class C:

if bool():
f()
1 + '' # E: Unsupported left operand type for + ("int")
1 + '' # E: Unsupported operand types for + ("int" and "str")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, I'd prefer using 1() instead (and not changing the fixture).

[builtins fixtures/list.pyi]
[out]

Expand All @@ -1992,6 +1992,6 @@ class C:

if bool():
f([])
1 + '' # E: Unsupported left operand type for + ("int")
1 + '' # E: Unsupported operand types for + ("int" and "str")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above.

[builtins fixtures/list.pyi]
[out]
2 changes: 1 addition & 1 deletion test-data/unit/check-modules.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1312,7 +1312,7 @@ class A:
def f(self) -> str: return 'foo'
class B(A):
def f(self) -> str: return self.x
def initialize(self): self.x = 'bar'
def initialize(self) -> None: self.x = 'bar'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having a return type may have well been intentional. I'd prefer either keeping the original test case or adding another variant with a return type annotation.

[out]


Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-statements.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ a @= 1 # E: Argument 1 to "__imatmul__" of "A" has incompatible type "int"; exp

[case testInplaceSetitem]
class A(object):
def __init__(self):
def __init__(self) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

self.a = 0

def __iadd__(self, a):
Expand Down
3 changes: 2 additions & 1 deletion test-data/unit/fixtures/list.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ class list(Generic[T]):

class tuple(Generic[T]): pass
class function: pass
class int: pass
class int:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed above, I wouldn't change the fixture.

def __add__(self, other: 'int') -> 'int': pass
class float: pass
class str: pass
class bool(int): pass
Expand Down