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.

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

chore(python): refactor unit / system test dependency install #1186

Closed
wants to merge 4 commits into from
Closed

chore(python): refactor unit / system test dependency install #1186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2532c21
chore(python): refactor unit / system test dependency install
tseaver Aug 18, 2021
706009e
chore: blacken
tseaver Aug 18, 2021
5bdc0bd
chore: avoid trailing whitespace
tseaver Aug 18, 2021
d2cf74c
chore: use editable installs for local deps
tseaver Aug 18, 2021
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
147 changes: 104 additions & 43 deletions synthtool/gcp/templates/python_library/noxfile.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,58 @@ import shutil

import nox


BLACK_VERSION = "black==19.10b0"
BLACK_PATHS = ["docs", "google", "tests", "noxfile.py", "setup.py"]

DEFAULT_PYTHON_VERSION="{{ default_python_version }}"
SYSTEM_TEST_PYTHON_VERSIONS=[{% for v in system_test_python_versions %}"{{v}}"{% if not loop.last %},{% endif %}{% endfor %}]
UNIT_TEST_PYTHON_VERSIONS=[{% for v in unit_test_python_versions %}"{{v}}"{% if not loop.last %},{% endif %}{% endfor %}]

UNIT_TEST_PYTHON_VERSIONS=[{% for v in unit_test_python_versions %}"{{v}}"{% if not loop.last %}, {% endif %}{% endfor %}]
UNIT_TEST_STANDARD_DEPENDENCIES = [
"mock",
"asyncmock",
"pytest",
"pytest-cov",
"pytest-asyncio",
]
UNIT_TEST_EXTERNAL_DEPENDENCIES = [{% for v in unit_test_external_dependencies %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to deprecate unit_test_external_dependencies? It seems quire redundant to me with unit_test_dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3ed5783

"{{v}}",{% endfor %}
]
UNIT_TEST_LOCAL_DEPENDENCIES = [{% for v in unit_test_local_dependencies %}
"{{v}}",{% endfor %}
]
UNIT_TEST_DEPENDENCIES = [{% for v in unit_test_dependencies %}
"{{v}}",{% endfor %}
]
UNIT_TEST_EXTRAS = [{% for v in unit_test_extras %}
"{{v}}",{% endfor %}
]
UNIT_TEST_EXTRAS_BY_PYTHON = {{ '{' }}{% if unit_test_extras_by_python %}{% for python_version, extras in unit_test_extras_by_python.items() %}
"{{python_version}}": [{% for v in extras %}
"{{v}}",{% endfor %}
],{% endfor %}{% endif %}
}

SYSTEM_TEST_PYTHON_VERSIONS=[{% for v in system_test_python_versions %}"{{v}}"{% if not loop.last %}, {% endif %}{% endfor %}]
SYSTEM_TEST_STANDARD_DEPENDENCIES = [
"mock", "pytest", "google-cloud-testutils",
]
SYSTEM_TEST_EXTERNAL_DEPENDENCIES = [{% for v in system_test_external_dependencies %}
"{{v}}",{% endfor %}
]
SYSTEM_TEST_LOCAL_DEPENDENCIES = [{% for v in system_test_local_dependencies %}
"{{v}}",{% endfor %}
]
SYSTEM_TEST_DEPENDENCIES = [{% for v in system_test_dependencies %}
"{{v}}",{% endfor %}
]
SYSTEM_TEST_EXTRAS = [{% for v in system_test_extras %}
"{{v}}",{% endfor %}
]
SYSTEM_TEST_EXTRAS_BY_PYTHON = {{ '{' }}{% if system_test_extras_by_python %}{% for python_version, extras in system_test_extras_by_python.items() %}
"{{python_version}}": [{% for v in extras %}
"{{v}}",{% endfor %}
],{% endfor %}{% endif %}
}

CURRENT_DIRECTORY = pathlib.Path(__file__).parent.absolute()

Expand Down Expand Up @@ -81,29 +126,38 @@ def lint_setup_py(session):
session.run("python", "setup.py", "check", "--restructuredtext", "--strict")


def install_unittest_dependencies(session, *constraints):
session.install(*UNIT_TEST_STANDARD_DEPENDENCIES, *constraints)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer if we installed UNIT_TEST_STANDARD_DEPENDENCIES and UNIT_TEST_DEPENDENCIES all in one go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure I see why? The prior template alwasy installed the "standard" deps ("mock", "asyncmock", "pytest", "pytest-cov", "pytest-asyncio") in a separate step, prior to any others.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason is that the pip resolver can run in one go. Otherwise subsequent pip installs might have to downgrade some packages.


if UNIT_TEST_EXTERNAL_DEPENDENCIES:
session.install(*UNIT_TEST_EXTERNAL_DEPENDENCIES, *constraints)

if UNIT_TEST_LOCAL_DEPENDENCIES:
session.install("-e", *UNIT_TEST_LOCAL_DEPENDENCIES, *constraints)

if UNIT_TEST_DEPENDENCIES:
session.install("-e", *UNIT_TEST_DEPENDENCIES, *constraints)
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't supposed to be installed in "editable" mode, are they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Likely not -- I doubt anybody is using a git: URL for a dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d274ce3


if UNIT_TEST_EXTRAS_BY_PYTHON:
extras = UNIT_TEST_EXTRAS_BY_PYTHON.get(session.python, [])
elif UNIT_TEST_EXTRAS:
extras = UNIT_TEST_EXTRAS
else:
extras = []

if extras:
session.install("-e", f".[{','.join(extras)}]", *constraints)
else:
session.install("-e", ".", *constraints)


def default(session):
# Install all test dependencies, then install this package in-place.

constraints_path = str(
CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt"
)
session.install("mock", "asyncmock", "pytest", "pytest-cov", "pytest-asyncio", "-c", constraints_path)
{% for d in unit_test_external_dependencies %}session.install("{{d}}", "-c", constraints_path){% endfor %}
{% for dependency in unit_test_local_dependencies %}session.install("-e", "{{dependency}}", "-c", constraints_path)
{% endfor %}
{% for dependency in unit_test_dependencies %}session.install("-e", "{{dependency}}", "-c", constraints_path){% endfor %}
{%- if unit_test_extras_by_python %}
{% for extras_python in unit_test_extras_by_python %}
{%- if not loop.first %}el{% endif %}if session.python == "{{extras_python}}":
extras = "[{{",".join(unit_test_extras_by_python[extras_python])}}]"
{% endfor %}else:
extras = "{%- if unit_test_extras %}[{{",".join(unit_test_extras)}}]{% endif %}"
session.install("-e", f".{extras}", "-c", constraints_path)
{% elif unit_test_extras %}
session.install("-e", ".[{{",".join(unit_test_extras)}}]", "-c", constraints_path)
{% else %}
session.install("-e", ".", "-c", constraints_path)
{% endif %}
install_unittest_dependencies(session, "-c", constraints_path)

# Run py.test against the unit tests.
session.run(
Expand All @@ -126,6 +180,35 @@ def unit(session):
default(session)


def install_systemtest_dependencies(session, *constraints):

# Use pre-release gRPC for system tests.
session.install("--pre", "grpcio")

session.install(*SYSTEM_TEST_STANDARD_DEPENDENCIES, *constraints)

if SYSTEM_TEST_EXTERNAL_DEPENDENCIES:
session.install(*SYSTEM_TEST_EXTERNAL_DEPENDENCIES, *constraints)

if SYSTEM_TEST_LOCAL_DEPENDENCIES:
session.install("-e", *SYSTEM_TEST_LOCAL_DEPENDENCIES, *constraints)

if SYSTEM_TEST_DEPENDENCIES:
session.install("-e", *SYSTEM_TEST_DEPENDENCIES, *constraints)

if SYSTEM_TEST_EXTRAS_BY_PYTHON:
extras = SYSTEM_TEST_EXTRAS_BY_PYTHON.get(session.python, [])
elif SYSTEM_TEST_EXTRAS:
extras = SYSTEM_TEST_EXTRAS
else:
extras = []

if extras:
session.install("-e", f".[{','.join(extras)}]", *constraints)
else:
session.install("-e", ".", *constraints)


@nox.session(python=SYSTEM_TEST_PYTHON_VERSIONS)
def system(session):
"""Run the system test suite."""
Expand All @@ -148,29 +231,7 @@ def system(session):
if not system_test_exists and not system_test_folder_exists:
session.skip("System tests were not found")

# Use pre-release gRPC for system tests.
session.install("--pre", "grpcio")

# Install all test dependencies, then install this package into the
# virtualenv's dist-packages.
session.install("mock", "pytest", "google-cloud-testutils"{% for d in system_test_external_dependencies %}, "{{d}}"{% endfor %}, "-c", constraints_path)

{%- if system_test_local_dependencies %}
{% for dependency in system_test_local_dependencies %}session.install("-e", "{{dependency}}", "-c", constraints_path)
{% endfor %}
{%- endif %}
{%- if system_test_extras_by_python %}
{% for extras_python in system_test_extras_by_python %}
{%- if not loop.first %}el{% endif %}if session.python == "{{extras_python}}":
extras = "[{{",".join(system_test_extras_by_python[extras_python])}}]"
{% endfor %}else:
extras = "{%- if system_test_extras %}[{{",".join(system_test_extras)}}]{% endif %}"
session.install("-e", f".{extras}", "-c", constraints_path)
{% elif system_test_extras %}
session.install("-e", ".[{{",".join(system_test_extras)}}]", "-c", constraints_path)
{% else %}
session.install("-e", ".", "-c", constraints_path)
{% endif %}
install_systemtest_dependencies(session, "-c", constraints_path)

# Run py.test against the system tests.
if system_test_exists:
Expand Down
80 changes: 61 additions & 19 deletions tests/test_python_library.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,43 +28,67 @@
@pytest.mark.parametrize(
["template_kwargs", "expected_text"],
[
({}, ["import nox", 'session.install("-e", ".", "-c", constraints_path)']),
({}, ["import nox", 'session.install("-e", ".", *constraints)']),
(
{"unit_test_local_dependencies": ["../testutils", "../unitutils"]},
[
'session.install("-e", "../testutils", "-c", constraints_path)',
'session.install("-e", "../unitutils", "-c", constraints_path)',
"""\
UNIT_TEST_LOCAL_DEPENDENCIES = [
"../testutils",
"../unitutils",
]""",
],
),
(
{"system_test_local_dependencies": ["../testutils", "../sysutils"]},
[
'session.install("-e", "../testutils", "-c", constraints_path)',
'session.install("-e", "../sysutils", "-c", constraints_path)',
"""\
SYSTEM_TEST_LOCAL_DEPENDENCIES = [
"../testutils",
"../sysutils",
]""",
],
),
(
{"unit_test_extras": ["abc", "def"]},
['session.install("-e", ".[abc,def]", "-c", constraints_path)'],
[
"""\
UNIT_TEST_EXTRAS = [
"abc",
"def",
]""",
],
),
(
{"system_test_extras": ["abc", "def"]},
['session.install("-e", ".[abc,def]", "-c", constraints_path)'],
"""\
SYSTEM_TEST_EXTRAS = [
"abc",
"def",
]""",
),
(
{"unit_test_extras_by_python": {"3.8": ["abc", "def"]}},
[
'if session.python == "3.8":\n extras = "[abc,def]"',
'else:\n extras = ""',
'session.install("-e", f".{extras}", "-c", constraints_path)',
"""\
UNIT_TEST_EXTRAS_BY_PYTHON = {
"3.8": [
"abc",
"def",
],
}""",
],
),
(
{"system_test_extras_by_python": {"3.8": ["abc", "def"]}},
[
'if session.python == "3.8":\n extras = "[abc,def]"',
'else:\n extras = ""',
'session.install("-e", f".{extras}", "-c", constraints_path)',
"""\
SYSTEM_TEST_EXTRAS_BY_PYTHON = {
"3.8": [
"abc",
"def",
],
}""",
],
),
(
Expand All @@ -73,9 +97,18 @@
"unit_test_extras_by_python": {"3.8": ["abc", "def"]},
},
[
'if session.python == "3.8":\n extras = "[abc,def]"',
'else:\n extras = "[tuv,wxyz]"',
'session.install("-e", f".{extras}", "-c", constraints_path)',
"""\
UNIT_TEST_EXTRAS = [
"tuv",
"wxyz",
]""",
"""\
UNIT_TEST_EXTRAS_BY_PYTHON = {
"3.8": [
"abc",
"def",
],
}""",
],
),
(
Expand All @@ -84,9 +117,18 @@
"system_test_extras_by_python": {"3.8": ["abc", "def"]},
},
[
'if session.python == "3.8":\n extras = "[abc,def]"',
'else:\n extras = "[tuv,wxyz]"',
'session.install("-e", f".{extras}", "-c", constraints_path)',
"""\
SYSTEM_TEST_EXTRAS = [
"tuv",
"wxyz",
]""",
"""\
SYSTEM_TEST_EXTRAS_BY_PYTHON = {
"3.8": [
"abc",
"def",
],
}""",
],
),
],
Expand Down