Skip to content

Commit

Permalink
Unify test suites' module exclusion logic (#14575)
Browse files Browse the repository at this point in the history
Individual test suites grew to have distinct approaches for selecting or
excluding modules / symbols, with distinct ad-hoc exclude lists. Rather
than rely on a common set of rules, test authors have been adding to
those distinct lists (having to modify the test suite to support their
test case).

In this PR, we will consolidate this logic so we could have a common set
of rules:

- The "tested corpus" is what's asserted on. Everything else has a
supporting role but does not contribute to what's being asserted on.
- The "tested corpus" is
  - the `__main__` module
- the `tested_modules`: modules provided through `[file ...]`, `[outfile
...]` or `[outfile-re ...]`
- It follows that library code, whether imported from lib-stub/ or
provided through `[builtins ...]` or `[typing ...]` will not be part of
the "tested corpus".
- At times we want `[file ...]` to also only have a supporting role and
not be part of the tested corpus. In tests we used to have conventions
like excluding modules starting with `_`. Instead, we'd have an explicit
`[fixture ...]` block that just like `[file ...]` except it doesn't
participate in the "tested corpus".
  • Loading branch information
ikonst authored Mar 2, 2023
1 parent bed49ab commit 099500e
Show file tree
Hide file tree
Showing 19 changed files with 173 additions and 230 deletions.
4 changes: 2 additions & 2 deletions mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,8 @@ def __init__(
self.quickstart_state = read_quickstart_file(options, self.stdout)
# Fine grained targets (module top levels and top level functions) processed by
# the semantic analyzer, used only for testing. Currently used only by the new
# semantic analyzer.
self.processed_targets: list[str] = []
# semantic analyzer. Tuple of module and target name.
self.processed_targets: list[tuple[str, str]] = []
# Missing stub packages encountered.
self.missing_stub_packages: set[str] = set()
# Cache for mypy ASTs that have completed semantic analysis
Expand Down
7 changes: 4 additions & 3 deletions mypy/semanal_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def process_top_levels(graph: Graph, scc: list[str], patches: Patches) -> None:
state = graph[next_id]
assert state.tree is not None
deferred, incomplete, progress = semantic_analyze_target(
next_id, state, state.tree, None, final_iteration, patches
next_id, next_id, state, state.tree, None, final_iteration, patches
)
all_deferred += deferred
any_progress = any_progress or progress
Expand Down Expand Up @@ -289,7 +289,7 @@ def process_top_level_function(
# OK, this is one last pass, now missing names will be reported.
analyzer.incomplete_namespaces.discard(module)
deferred, incomplete, progress = semantic_analyze_target(
target, state, node, active_type, final_iteration, patches
target, module, state, node, active_type, final_iteration, patches
)
if final_iteration:
assert not deferred, "Must not defer during final iteration"
Expand Down Expand Up @@ -318,6 +318,7 @@ def get_all_leaf_targets(file: MypyFile) -> list[TargetInfo]:

def semantic_analyze_target(
target: str,
module: str,
state: State,
node: MypyFile | FuncDef | OverloadedFuncDef | Decorator,
active_type: TypeInfo | None,
Expand All @@ -331,7 +332,7 @@ def semantic_analyze_target(
- was some definition incomplete (need to run another pass)
- were any new names defined (or placeholders replaced)
"""
state.manager.processed_targets.append(target)
state.manager.processed_targets.append((module, target))
tree = state.tree
assert tree is not None
analyzer = state.manager.semantic_analyzer
Expand Down
29 changes: 22 additions & 7 deletions mypy/test/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ class DeleteFile(NamedTuple):
FileOperation: _TypeAlias = Union[UpdateFile, DeleteFile]


def _file_arg_to_module(filename: str) -> str:
filename, _ = os.path.splitext(filename)
parts = filename.split("/") # not os.sep since it comes from test data
if parts[-1] == "__init__":
parts.pop()
return ".".join(parts)


def parse_test_case(case: DataDrivenTestCase) -> None:
"""Parse and prepare a single case from suite with test case descriptions.
Expand All @@ -65,22 +73,26 @@ def parse_test_case(case: DataDrivenTestCase) -> None:
rechecked_modules: dict[int, set[str]] = {} # from run number module names
triggered: list[str] = [] # Active triggers (one line per incremental step)
targets: dict[int, list[str]] = {} # Fine-grained targets (per fine-grained update)
test_modules: list[str] = [] # Modules which are deemed "test" (vs "fixture")

# Process the parsed items. Each item has a header of form [id args],
# optionally followed by lines of text.
item = first_item = test_items[0]
test_modules.append("__main__")
for item in test_items[1:]:
if item.id in {"file", "outfile", "outfile-re"}:
if item.id in {"file", "fixture", "outfile", "outfile-re"}:
# Record an extra file needed for the test case.
assert item.arg is not None
contents = expand_variables("\n".join(item.data))
file_entry = (join(base_path, item.arg), contents)
if item.id == "file":
files.append(file_entry)
path = join(base_path, item.arg)
if item.id != "fixture":
test_modules.append(_file_arg_to_module(item.arg))
if item.id in {"file", "fixture"}:
files.append((path, contents))
elif item.id == "outfile-re":
output_files.append((file_entry[0], re.compile(file_entry[1].rstrip(), re.S)))
else:
output_files.append(file_entry)
output_files.append((path, re.compile(contents.rstrip(), re.S)))
elif item.id == "outfile":
output_files.append((path, contents))
elif item.id == "builtins":
# Use an alternative stub file for the builtins module.
assert item.arg is not None
Expand Down Expand Up @@ -207,6 +219,7 @@ def parse_test_case(case: DataDrivenTestCase) -> None:
case.triggered = triggered or []
case.normalize_output = normalize_output
case.expected_fine_grained_targets = targets
case.test_modules = test_modules


class DataDrivenTestCase(pytest.Item):
Expand All @@ -225,6 +238,8 @@ class DataDrivenTestCase(pytest.Item):

# (file path, file content) tuples
files: list[tuple[str, str]]
# Modules which is to be considered "test" rather than "fixture"
test_modules: list[str]
expected_stale_modules: dict[int, set[str]]
expected_rechecked_modules: dict[int, set[str]]
expected_fine_grained_targets: dict[int, list[str]]
Expand Down
9 changes: 3 additions & 6 deletions mypy/test/testcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from mypy.errors import CompileError
from mypy.modulefinder import BuildSource, FindModuleCache, SearchPaths
from mypy.options import TYPE_VAR_TUPLE, UNPACK
from mypy.semanal_main import core_modules
from mypy.test.config import test_data_prefix, test_temp_dir
from mypy.test.data import DataDrivenTestCase, DataSuite, FileOperation, module_from_path
from mypy.test.helpers import (
Expand Down Expand Up @@ -188,12 +187,10 @@ def run_case_once(
if incremental_step:
name += str(incremental_step + 1)
expected = testcase.expected_fine_grained_targets.get(incremental_step + 1)
actual = res.manager.processed_targets
# Skip the initial builtin cycle.
actual = [
t
for t in actual
if not any(t.startswith(mod) for mod in core_modules + ["mypy_extensions"])
target
for module, target in res.manager.processed_targets
if module in testcase.test_modules
]
if expected is not None:
assert_target_equivalence(name, expected, actual)
Expand Down
13 changes: 3 additions & 10 deletions mypy/test/testdeps.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,9 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
a = ["Unknown compile error (likely syntax error in test case or fixture)"]
else:
deps: defaultdict[str, set[str]] = defaultdict(set)
for module in files:
if (
module in dumped_modules
or dump_all
and module
not in ("abc", "typing", "mypy_extensions", "typing_extensions", "enum")
):
new_deps = get_dependencies(
files[module], type_map, options.python_version, options
)
for module, file in files.items():
if (module in dumped_modules or dump_all) and (module in testcase.test_modules):
new_deps = get_dependencies(file, type_map, options.python_version, options)
for source in new_deps:
deps[source].update(new_deps[source])

Expand Down
46 changes: 13 additions & 33 deletions mypy/test/testmerge.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,6 @@
AST = "AST"


NOT_DUMPED_MODULES = (
"builtins",
"typing",
"abc",
"contextlib",
"sys",
"mypy_extensions",
"typing_extensions",
"enum",
)


class ASTMergeSuite(DataSuite):
files = ["merge.test"]

Expand Down Expand Up @@ -84,13 +72,13 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
target_path = os.path.join(test_temp_dir, "target.py")
shutil.copy(os.path.join(test_temp_dir, "target.py.next"), target_path)

a.extend(self.dump(fine_grained_manager, kind))
a.extend(self.dump(fine_grained_manager, kind, testcase.test_modules))
old_subexpr = get_subexpressions(result.manager.modules["target"])

a.append("==>")

new_file, new_types = self.build_increment(fine_grained_manager, "target", target_path)
a.extend(self.dump(fine_grained_manager, kind))
a.extend(self.dump(fine_grained_manager, kind, testcase.test_modules))

for expr in old_subexpr:
if isinstance(expr, TypeVarExpr):
Expand Down Expand Up @@ -137,34 +125,32 @@ def build_increment(
type_map = manager.graph[module_id].type_map()
return module, type_map

def dump(self, manager: FineGrainedBuildManager, kind: str) -> list[str]:
modules = manager.manager.modules
def dump(
self, manager: FineGrainedBuildManager, kind: str, test_modules: list[str]
) -> list[str]:
modules = {
name: file for name, file in manager.manager.modules.items() if name in test_modules
}
if kind == AST:
return self.dump_asts(modules)
elif kind == TYPEINFO:
return self.dump_typeinfos(modules)
elif kind == SYMTABLE:
return self.dump_symbol_tables(modules)
elif kind == TYPES:
return self.dump_types(manager)
return self.dump_types(modules, manager)
assert False, f"Invalid kind {kind}"

def dump_asts(self, modules: dict[str, MypyFile]) -> list[str]:
a = []
for m in sorted(modules):
if m in NOT_DUMPED_MODULES:
# We don't support incremental checking of changes to builtins, etc.
continue
s = modules[m].accept(self.str_conv)
a.extend(s.splitlines())
return a

def dump_symbol_tables(self, modules: dict[str, MypyFile]) -> list[str]:
a = []
for id in sorted(modules):
if not is_dumped_module(id):
# We don't support incremental checking of changes to builtins, etc.
continue
a.extend(self.dump_symbol_table(id, modules[id].names))
return a

Expand Down Expand Up @@ -197,8 +183,6 @@ def format_symbol_table_node(self, node: SymbolTableNode) -> str:
def dump_typeinfos(self, modules: dict[str, MypyFile]) -> list[str]:
a = []
for id in sorted(modules):
if not is_dumped_module(id):
continue
a.extend(self.dump_typeinfos_recursive(modules[id].names))
return a

Expand All @@ -217,13 +201,13 @@ def dump_typeinfo(self, info: TypeInfo) -> list[str]:
s = info.dump(str_conv=self.str_conv, type_str_conv=self.type_str_conv)
return s.splitlines()

def dump_types(self, manager: FineGrainedBuildManager) -> list[str]:
def dump_types(
self, modules: dict[str, MypyFile], manager: FineGrainedBuildManager
) -> list[str]:
a = []
# To make the results repeatable, we try to generate unique and
# deterministic sort keys.
for module_id in sorted(manager.manager.modules):
if not is_dumped_module(module_id):
continue
for module_id in sorted(modules):
all_types = manager.manager.all_types
# Compute a module type map from the global type map
tree = manager.graph[module_id].tree
Expand All @@ -242,7 +226,3 @@ def dump_types(self, manager: FineGrainedBuildManager) -> list[str]:

def format_type(self, typ: Type) -> str:
return typ.accept(self.type_str_conv)


def is_dumped_module(id: str) -> bool:
return id not in NOT_DUMPED_MODULES and (not id.startswith("_") or id == "__main__")
54 changes: 16 additions & 38 deletions mypy/test/testsemanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from __future__ import annotations

import os.path
import sys
from typing import Dict

Expand Down Expand Up @@ -77,27 +76,9 @@ def test_semanal(testcase: DataDrivenTestCase) -> None:
raise CompileError(a)
# Include string representations of the source files in the actual
# output.
for fnam in sorted(result.files.keys()):
f = result.files[fnam]
# Omit the builtins module and files with a special marker in the
# path.
# TODO the test is not reliable
if (
not f.path.endswith(
(
os.sep + "builtins.pyi",
"typing.pyi",
"mypy_extensions.pyi",
"typing_extensions.pyi",
"abc.pyi",
"collections.pyi",
"sys.pyi",
)
)
and not os.path.basename(f.path).startswith("_")
and not os.path.splitext(os.path.basename(f.path))[0].endswith("_")
):
a += str(f).split("\n")
for module in sorted(result.files.keys()):
if module in testcase.test_modules:
a += str(result.files[module]).split("\n")
except CompileError as e:
a = e.messages
if testcase.normalize_output:
Expand Down Expand Up @@ -164,10 +145,10 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:
a = result.errors
if a:
raise CompileError(a)
for f in sorted(result.files.keys()):
if f not in ("builtins", "typing", "abc"):
a.append(f"{f}:")
for s in str(result.files[f].names).split("\n"):
for module in sorted(result.files.keys()):
if module in testcase.test_modules:
a.append(f"{module}:")
for s in str(result.files[module].names).split("\n"):
a.append(" " + s)
except CompileError as e:
a = e.messages
Expand Down Expand Up @@ -199,11 +180,13 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:

# Collect all TypeInfos in top-level modules.
typeinfos = TypeInfoMap()
for f in result.files.values():
for n in f.names.values():
if isinstance(n.node, TypeInfo):
assert n.fullname
typeinfos[n.fullname] = n.node
for module, file in result.files.items():
if module in testcase.test_modules:
for n in file.names.values():
if isinstance(n.node, TypeInfo):
assert n.fullname
if any(n.fullname.startswith(m + ".") for m in testcase.test_modules):
typeinfos[n.fullname] = n.node

# The output is the symbol table converted into a string.
a = str(typeinfos).split("\n")
Expand All @@ -220,12 +203,7 @@ class TypeInfoMap(Dict[str, TypeInfo]):
def __str__(self) -> str:
a: list[str] = ["TypeInfoMap("]
for x, y in sorted(self.items()):
if (
not x.startswith("builtins.")
and not x.startswith("typing.")
and not x.startswith("abc.")
):
ti = ("\n" + " ").join(str(y).split("\n"))
a.append(f" {x} : {ti}")
ti = ("\n" + " ").join(str(y).split("\n"))
a.append(f" {x} : {ti}")
a[-1] += ")"
return "\n".join(a)
27 changes: 4 additions & 23 deletions mypy/test/testtransform.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

from __future__ import annotations

import os.path

from mypy import build
from mypy.errors import CompileError
from mypy.modulefinder import BuildSource
Expand Down Expand Up @@ -50,29 +48,12 @@ def test_transform(testcase: DataDrivenTestCase) -> None:
raise CompileError(a)
# Include string representations of the source files in the actual
# output.
for fnam in sorted(result.files.keys()):
f = result.files[fnam]

# Omit the builtins module and files with a special marker in the
# path.
# TODO the test is not reliable
if (
not f.path.endswith(
(
os.sep + "builtins.pyi",
"typing_extensions.pyi",
"typing.pyi",
"abc.pyi",
"sys.pyi",
)
)
and not os.path.basename(f.path).startswith("_")
and not os.path.splitext(os.path.basename(f.path))[0].endswith("_")
):
for module in sorted(result.files.keys()):
if module in testcase.test_modules:
t = TypeAssertTransformVisitor()
t.test_only = True
f = t.mypyfile(f)
a += str(f).split("\n")
file = t.mypyfile(result.files[module])
a += str(file).split("\n")
except CompileError as e:
a = e.messages
if testcase.normalize_output:
Expand Down
Loading

0 comments on commit 099500e

Please sign in to comment.