Skip to content
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

Add more typing to the codebase and various fixes. #285

Merged
merged 1 commit into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
- name: Install
run: |
python -m pip install --upgrade pip
pip install --upgrade .[dev]
pip install --upgrade '.[dev]'

- name: Format
run: black --check --diff green example
Expand Down
17 changes: 15 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# SHELL ensures more consistent behavior between OSes.
SHELL=/bin/bash

VERSION=$(shell cat green/VERSION)


clean: clean-message clean-silent

clean-message:
Expand All @@ -22,7 +26,7 @@ test: test-versions test-installed test-coverage
@echo "\n(test) completed\n"

test-local:
@pip3 install --upgrade -e .[dev]
@pip3 install --upgrade -e '.[dev]'
@make test-installed
make test-versions
make test-coverage
Expand All @@ -32,9 +36,15 @@ test-on-containers: clean-silent
@# Run the tests on pristine containers to isolate us from the local environment.
@for version in 3.8 3.9 3.10 3.11 3.12.0; do \
docker run --rm -it -v `pwd`:/green python:$$version \
bash -c "python --version; cd /green && pip install -e .[dev] && ./g green" ; \
bash -c "python --version; cd /green && pip install -e '.[dev]' && ./g green" ; \
done

test-coverage-on-container: clean-silent
@# Run the tests on pristine containers to isolate us from the local environment.
docker run --rm -it -v `pwd`:/green python:3.12.0 \
bash -c "cd /green && pip install -e '.[dev]' && ./g 3 -r -vvvv green"


test-coverage:
@# Generate coverage files for travis builds (don't clean after this!)
@make clean-silent
Expand Down Expand Up @@ -95,3 +105,6 @@ release-unsafe:
twine upload --username CleanCut dist/green-$(VERSION).tar.gz

release: release-test release-tag release-unsafe

# Declare all targets as phony so that make will always run them.
.PHONY: $(MAKECMDGOALS)
2 changes: 1 addition & 1 deletion green/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4.0.0
4.0.1-dev
12 changes: 8 additions & 4 deletions green/cmdline.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
from __future__ import annotations


import os
import sys
import tempfile
from typing import Sequence

# Importing from green (other than config) is done after coverage initialization
import green.config as config


def _main(argv, testing):
def _main(argv: Sequence[str] | None, testing: bool) -> int:
args = config.parseArguments(argv)
args = config.mergeConfig(args, testing)

Expand Down Expand Up @@ -77,9 +81,9 @@ def _main(argv, testing):
return int(not result.wasSuccessful())


def main(argv=None, testing=False):
def main(argv: Sequence[str] | None = None, testing: bool = False):
# create the temp dir only once (i.e., not while in the recursed call)
if os.environ.get("TMPDIR") is None: # pragma: nocover
if not os.environ.get("TMPDIR"): # pragma: nocover
try:
with tempfile.TemporaryDirectory() as temp_dir_for_tests:
try:
Expand All @@ -94,7 +98,7 @@ def main(argv=None, testing=False):
# "Directory not empty" when trying to delete the temp dir can just be a warning
print(f"warning: {os_error.strerror}")
else:
raise (os_error)
raise os_error
else:
return _main(argv, testing)

Expand Down
2 changes: 2 additions & 0 deletions green/command.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import sys

from setuptools import Command
Expand Down
7 changes: 6 additions & 1 deletion green/config.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
# We have to use this entire file before we can turn coverage on, so we exclude
# it from coverage. We still have tests, though!

from __future__ import annotations

import argparse # pragma: no cover
import configparser # pragma: no cover
from typing import Sequence

import coverage # pragma: no cover

Expand Down Expand Up @@ -77,7 +80,9 @@ def __call__(self, action):
self.options.extend(action.option_strings[0:2])


def parseArguments(argv=None): # pragma: no cover
def parseArguments(
argv: Sequence[str] | None = None,
) -> argparse.Namespace: # pragma: no cover
"""
I parse arguments in sys.argv and return the args object. The parser
itself is available as args.parser.
Expand Down
2 changes: 2 additions & 0 deletions green/djangorunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
TEST_RUNNER="green.djangorunner.DjangoRunner"
"""

from __future__ import annotations

from argparse import Namespace
import os
import sys
Expand Down
2 changes: 2 additions & 0 deletions green/examples.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import sys
import unittest

Expand Down
3 changes: 3 additions & 0 deletions green/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
from __future__ import annotations


class InitializerOrFinalizerError(Exception):
pass
2 changes: 2 additions & 0 deletions green/junit.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

from lxml.etree import Element, tostring as to_xml


Expand Down
95 changes: 57 additions & 38 deletions green/loader.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from collections import OrderedDict
import pathlib
from doctest import DocTestSuite
from fnmatch import fnmatch
import functools
Expand All @@ -12,16 +12,27 @@
import sys
import unittest
import traceback
from typing import Iterable, TYPE_CHECKING

from green.output import debug
from green import result
from green.suite import GreenTestSuite

if TYPE_CHECKING:
from types import ModuleType
from unittest import TestSuite, TestCase

python_file_pattern = re.compile(r"^[_a-z]\w*?\.py$", re.IGNORECASE)
python_dir_pattern = re.compile(r"^[_a-z]\w*?$", re.IGNORECASE)


class GreenTestLoader(unittest.TestLoader):
"""
This class is responsible for loading tests according to various criteria
and returning them wrapped in a GreenTestSuite. The super class wraps with
TestSuite.
"""

suiteClass = GreenTestSuite

def loadTestsFromTestCase(self, testCaseClass):
Expand All @@ -44,7 +55,7 @@ def filter_test_methods(attrname):
test_case_names = ["runTest"]
return flattenTestSuite(map(testCaseClass, test_case_names))

def loadFromModuleFilename(self, filename):
def loadFromModuleFilename(self, filename: str):
dotted_module, parent_dir = findDottedModuleAndParentDir(filename)
# Adding the parent path of the module to the start of sys.path is
# the closest we can get to an absolute import in Python that I can
Expand Down Expand Up @@ -153,13 +164,15 @@ def discover(self, current_path, file_pattern="test*.py", top_level_dir=None):

return flattenTestSuite(suite) if suite.countTestCases() else None

def loadTargets(self, targets, file_pattern="test*.py"):
# If a string was passed in, put it into a list.
if type(targets) != list:
def loadTargets(
self, targets: Iterable[str] | str, file_pattern: str = "test*.py"
) -> GreenTestSuite | None:
# If a string was passed in, put it into a tuple.
if isinstance(targets, str):
targets = [targets]

# Make sure there are no duplicate entries, preserving order
target_dict = OrderedDict()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OrderedDict has more features but if they are not used, a regular dict is good enough.

target_dict = {}
for target in targets:
target_dict[target] = True
targets = target_dict.keys()
Expand Down Expand Up @@ -325,15 +338,18 @@ def toProtoTestList(
return test_list


def toParallelTargets(suite, targets: list[str]) -> list[str]:
def toParallelTargets(suite, targets: Iterable[str]) -> list[str]:
"""
Produce a list of targets which should be tested in parallel.

For the most part, this will be a list of test modules.
The exception is when a dotted name representing something more granular
than a module was input (like an individual test case or test method).
"""
targets = [x for x in targets if x != "."]
if isinstance(targets, str):
# This should not happen, but mypy treats str as a valid sequence of strings.
targets = (targets,)
targets = (x for x in targets if x != ".")
# First, convert the suite to a proto test list - proto tests nicely
# parse things like the fully dotted name of the test and the
# finest-grained module it belongs to, which simplifies our job.
Expand Down Expand Up @@ -371,9 +387,9 @@ def toParallelTargets(suite, targets: list[str]) -> list[str]:
return parallel_targets


def getCompletions(target):
def getCompletions(target: list[str] | str):
# This option expects 0 or 1 targets
if type(target) == list:
if not isinstance(target, str):
target = target[0]
if target == ".":
target = ""
Expand All @@ -382,11 +398,12 @@ def getCompletions(target):

# First try the completion as-is. It might be at a valid spot.
loader = GreenTestLoader()
# FIXME: We do not pass file_pattern here, ignoring `--file-pattern`?
test_suite = loader.loadTargets(target)
if not test_suite:
# Next, try stripping to the previous '.'
last_dot_idx = target.rfind(".")
to_complete = None
to_complete: str | Iterable[str] = ""
if last_dot_idx > 0:
to_complete = target[:last_dot_idx]
elif len(target):
Expand Down Expand Up @@ -415,43 +432,41 @@ def getCompletions(target):
dotted_name = dotted_name[:idx]
if dotted_name.startswith(target):
dotted_names.add(dotted_name)
else: # pragma: nocover -- occurs in Python <= 3.7, but can't find a reasonable way to cover this line when it doesn't in Python > 3.7
else: # pragma: no cover -- occurs in Python <= 3.7, but can't find a reasonable way to cover this line when it doesn't in Python > 3.7
break
return "\n".join(sorted(list(dotted_names)))


def isPackage(file_path):
def isPackage(file_path: pathlib.Path) -> bool:
"""
Determine whether or not a given path is a (sub)package or not.
"""
return os.path.isdir(file_path) and os.path.isfile(
os.path.join(file_path, "__init__.py")
)
return file_path.is_dir() and (file_path / "__init__.py").is_file()


def findDottedModuleAndParentDir(file_path):
def findDottedModuleAndParentDir(file_path: str | pathlib.Path) -> tuple[str, str]:
"""
I return a tuple (dotted_module, parent_dir) where dotted_module is the
full dotted name of the module with respect to the package it is in, and
parent_dir is the absolute path to the parent directory of the package.

If the python file is not part of a package, I return (None, None).

For for filepath /a/b/c/d.py where b is the package, ('b.c.d', '/a')
For filepath '/a/b/c/d.py' where b is the package, ('b.c.d', '/a')
would be returned.
"""
if not os.path.isfile(file_path):
path = pathlib.Path(file_path)
if not path.is_file():
raise ValueError(f"'{file_path}' is not a file.")
parent_dir = os.path.dirname(os.path.abspath(file_path))
dotted_module = os.path.basename(file_path).replace(".py", "")
parent_dir = path.absolute().parent
dotted_module = path.stem
while isPackage(parent_dir):
dotted_module = os.path.basename(parent_dir) + "." + dotted_module
parent_dir = os.path.dirname(parent_dir)
dotted_module = f"{parent_dir.stem}.{dotted_module}"
parent_dir = parent_dir.parent
debug(f"Dotted module: {parent_dir} -> {dotted_module}", 2)
return (dotted_module, parent_dir)
# TODO: consider returning the Path object for the parent directory.
return dotted_module, str(parent_dir)


def isTestCaseDisabled(test_case_class, method_name):
def isTestCaseDisabled(test_case_class: TestCase, method_name: str):
"""
I check to see if a method on a TestCase has been disabled via nose's
convention for disabling a TestCase. This makes it so that users can
Expand All @@ -461,24 +476,28 @@ def isTestCaseDisabled(test_case_class, method_name):
return getattr(test_method, "__test__", "not nose") is False


def flattenTestSuite(test_suite, module=None):
# Look for a `doctest_modules` list and attempt to add doctest tests to the
# suite of tests that we are about to flatten.
def flattenTestSuite(
test_suite: list[TestSuite] | TestSuite, module: ModuleType | None = None
) -> GreenTestSuite:
"""
Look for a `doctest_modules` list and attempt to add doctest tests to the
suite of tests that we are about to flatten.
"""
# todo: rename this function to something more appropriate.
suites = [test_suite]
suites = test_suite if isinstance(test_suite, list) else [test_suite]
doctest_modules = getattr(module, "doctest_modules", ())
for doctest_module in doctest_modules:
suite = DocTestSuite(doctest_module)
suite.injected_module = module.__name__
suites.append(suite)
doc_suite = DocTestSuite(doctest_module)
# Forcing an injected_module onto DocTestSuite.
doc_suite.injected_module = module.__name__ # type: ignore
suites.append(doc_suite)

# Now extract all tests from the suite hierarchies and flatten them into a
# single suite with all tests.
tests = []
tests: list[TestCase | TestSuite] = []
for suite in suites:
injected_module = None
if getattr(suite, "injected_module", None):
injected_module = suite.injected_module
# injected_module is not present in DocTestSuite.
injected_module: str | None = getattr(suite, "injected_module", None)
for test in suite:
if injected_module:
# For doctests, inject the test module name so we can later
Expand Down
Loading
Loading