-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
STYLE pre-commit check to ensure that test functions name starts with test #50397
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1323,10 +1323,6 @@ def test_period_can_hold_element(self, element): | |
elem = element(dti) | ||
self.check_series_setitem(elem, pi, False) | ||
|
||
def check_setting(self, elem, index: Index, inplace: bool): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. helper function which wasn't being used anymore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jbrockmendel looks like it was added in https://github.com/pandas-dev/pandas/pull/39120/files - do you remember if it was meant to be used anywhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it was meant to de-duplicate something but never got used. Rip it out! |
||
self.check_series_setitem(elem, index, inplace) | ||
self.check_frame_setitem(elem, index, inplace) | ||
|
||
def check_can_hold_element(self, obj, elem, inplace: bool): | ||
blk = obj._mgr.blocks[0] | ||
if inplace: | ||
|
@@ -1350,23 +1346,6 @@ def check_series_setitem(self, elem, index: Index, inplace: bool): | |
else: | ||
assert ser.dtype == object | ||
|
||
def check_frame_setitem(self, elem, index: Index, inplace: bool): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. helper function which wasn't being used anymore |
||
arr = index._data.copy() | ||
df = DataFrame(arr) | ||
|
||
self.check_can_hold_element(df, elem, inplace) | ||
|
||
if is_scalar(elem): | ||
df.iloc[0, 0] = elem | ||
else: | ||
df.iloc[: len(elem), 0] = elem | ||
|
||
if inplace: | ||
# assertion here implies setting was done inplace | ||
assert df._mgr.arrays[0] is arr | ||
else: | ||
assert df.dtypes[0] == object | ||
|
||
|
||
class TestShouldStore: | ||
def test_should_store_categorical(self): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,10 +113,11 @@ def test_read_columns(self): | |
columns = ["col1", "col3"] | ||
self.check_round_trip(df, expected=df[columns], columns=columns) | ||
|
||
def read_columns_different_order(self): | ||
def test_read_columns_different_order(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alimcmaster1 this test was never running because its name didn't start with |
||
# GH 33878 | ||
df = pd.DataFrame({"A": [1, 2], "B": ["x", "y"], "C": [True, False]}) | ||
self.check_round_trip(df, columns=["B", "A"]) | ||
expected = df[["B", "A"]] | ||
self.check_round_trip(df, expected, columns=["B", "A"]) | ||
|
||
def test_unsupported_other(self): | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,13 @@ | |
_testing as tm, | ||
concat, | ||
) | ||
from pandas.tests.strings.test_strings import assert_series_or_index_equal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function was defined in another file, but only used here. so, just moving it here |
||
|
||
|
||
def assert_series_or_index_equal(left, right): | ||
if isinstance(left, Series): | ||
tm.assert_series_equal(left, right) | ||
else: # Index | ||
tm.assert_index_equal(left, right) | ||
|
||
|
||
@pytest.mark.parametrize("other", [None, Series, Index]) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,13 +30,18 @@ | |
YearEnd, | ||
) | ||
|
||
from pandas.tests.tseries.offsets.test_offsets import get_utc_offset_hours | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. defined in another file, but only used here. so, let's just define it here |
||
from pandas.util.version import Version | ||
|
||
# error: Module has no attribute "__version__" | ||
pytz_version = Version(pytz.__version__) # type: ignore[attr-defined] | ||
|
||
|
||
def get_utc_offset_hours(ts): | ||
# take a Timestamp and compute total hours of utc offset | ||
o = ts.utcoffset() | ||
return (o.days * 24 * 3600 + o.seconds) / 3600.0 | ||
|
||
|
||
class TestDST: | ||
|
||
# one microsecond before the DST transition | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,152 @@ | ||
""" | ||
Check that test names start with `test`, and that test classes start with `Test`. | ||
|
||
This is meant to be run as a pre-commit hook - to run it manually, you can do: | ||
|
||
pre-commit run check-test-naming --all-files | ||
|
||
NOTE: if this finds a false positive, you can add the comment `# not a test` to the | ||
class or function definition. Though hopefully that shouldn't be necessary. | ||
""" | ||
from __future__ import annotations | ||
|
||
import argparse | ||
import ast | ||
import os | ||
from pathlib import Path | ||
import sys | ||
from typing import ( | ||
Iterator, | ||
Sequence, | ||
) | ||
|
||
PRAGMA = "# not a test" | ||
|
||
|
||
def _find_names(node: ast.Module) -> Iterator[str]: | ||
for _node in ast.walk(node): | ||
if isinstance(_node, ast.Name): | ||
yield _node.id | ||
elif isinstance(_node, ast.Attribute): | ||
yield _node.attr | ||
|
||
|
||
def _is_fixture(node: ast.expr) -> bool: | ||
if isinstance(node, ast.Call): | ||
node = node.func | ||
return ( | ||
isinstance(node, ast.Attribute) | ||
and node.attr == "fixture" | ||
and isinstance(node.value, ast.Name) | ||
and node.value.id == "pytest" | ||
) | ||
|
||
|
||
def _is_register_dtype(node): | ||
return isinstance(node, ast.Name) and node.id == "register_extension_dtype" | ||
|
||
|
||
def is_misnamed_test_func( | ||
node: ast.expr | ast.stmt, names: Sequence[str], line: str | ||
) -> bool: | ||
return ( | ||
isinstance(node, ast.FunctionDef) | ||
and not node.name.startswith("test") | ||
and names.count(node.name) == 0 | ||
and not any(_is_fixture(decorator) for decorator in node.decorator_list) | ||
and PRAGMA not in line | ||
and node.name | ||
not in ("teardown_method", "setup_method", "teardown_class", "setup_class") | ||
) | ||
|
||
|
||
def is_misnamed_test_class( | ||
node: ast.expr | ast.stmt, names: Sequence[str], line: str | ||
) -> bool: | ||
return ( | ||
isinstance(node, ast.ClassDef) | ||
and not node.name.startswith("Test") | ||
and names.count(node.name) == 0 | ||
and not any(_is_register_dtype(decorator) for decorator in node.decorator_list) | ||
and PRAGMA not in line | ||
) | ||
|
||
|
||
def main(content: str, file: str) -> int: | ||
lines = content.splitlines() | ||
tree = ast.parse(content) | ||
names = list(_find_names(tree)) | ||
ret = 0 | ||
for node in tree.body: | ||
if is_misnamed_test_func(node, names, lines[node.lineno - 1]): | ||
print( | ||
f"{file}:{node.lineno}:{node.col_offset} " | ||
"found test function which does not start with 'test'" | ||
) | ||
ret = 1 | ||
elif is_misnamed_test_class(node, names, lines[node.lineno - 1]): | ||
print( | ||
f"{file}:{node.lineno}:{node.col_offset} " | ||
"found test class which does not start with 'Test'" | ||
) | ||
ret = 1 | ||
if ( | ||
isinstance(node, ast.ClassDef) | ||
and names.count(node.name) == 0 | ||
and not any( | ||
_is_register_dtype(decorator) for decorator in node.decorator_list | ||
) | ||
and PRAGMA not in lines[node.lineno - 1] | ||
): | ||
for _node in node.body: | ||
if is_misnamed_test_func(_node, names, lines[_node.lineno - 1]): | ||
# It could be that this function is used somewhere by the | ||
# parent class. For example, there might be a base class | ||
# with | ||
# | ||
# class Foo: | ||
# def foo(self): | ||
# assert 1+1==2 | ||
# def test_foo(self): | ||
# self.foo() | ||
# | ||
# and then some subclass overwrites `foo`. So, we check that | ||
# `self.foo` doesn't appear in any of the test classes. | ||
# Note some false negatives might get through, but that's OK. | ||
# This is good enough that has helped identify several examples | ||
# of tests not being run. | ||
assert isinstance(_node, ast.FunctionDef) # help mypy | ||
should_continue = False | ||
for _file in (Path("pandas") / "tests").rglob("*.py"): | ||
with open(os.path.join(_file)) as fd: | ||
_content = fd.read() | ||
if f"self.{_node.name}" in _content: | ||
should_continue = True | ||
break | ||
if should_continue: | ||
continue | ||
|
||
print( | ||
f"{file}:{_node.lineno}:{_node.col_offset} " | ||
"found test function which does not start with 'test'" | ||
) | ||
ret = 1 | ||
return ret | ||
|
||
|
||
if __name__ == "__main__": | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument("paths", nargs="*") | ||
args = parser.parse_args() | ||
|
||
ret = 0 | ||
|
||
for file in args.paths: | ||
filename = os.path.basename(file) | ||
if not (filename.startswith("test") and filename.endswith(".py")): | ||
continue | ||
with open(file, encoding="utf-8") as fd: | ||
content = fd.read() | ||
ret |= main(content, file) | ||
|
||
sys.exit(ret) |
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.
these exclusions will be removed as they're addressed