Repository: googleapis/synthtool

Issue:
Python: noxfile support for extras
We have the following template variables for installing dependencies:

* `unit_test_external_dependencies` -- install package from PyPI
* `unit_test_local_dependencies` -- install package (in editable mode) from local nearby git (not relevant for non-mono-repo?)
* `unit_test_dependencies` -- same as `unit_test_local_dependencies`?
* `system_test_external_dependencies` -- install package from PyPI
* `system_test_local_dependencies` -- install package (in editable mode) from local nearby git (not relevant for non-mono-repo?)

In BigQuery, we actually use  `system_test_local_dependencies` and `unit_test_local_dependencies` to install "extras", but then we have logic to remove the redundant `session.install("-e", ".")` (see: https://github.com/googleapis/python-bigquery-storage/pull/93)

I propose the following additional template parameters:

* `unit_test_extras_dependencies`
* `system_test_extras_dependencies`

When these are populated, it'll change `session.install("-e", ".")` to `session.install("-e", ".[dep1,dep2]")`.

Discussion:
tswast: In addition to the unit tests, I've tried these new parameters out here: https://github.com/googleapis/python-bigquery-storage/pull/118
SurferJeffAtGoogle: Tim, Bu Sun approved, so please merge at your earliest convenience.
tswast: @SurferJeffAtGoogle  It looks like Bu Sun isn't in CODEOWNERS. I can't merge.

![image](https://user-images.githubusercontent.com/247555/105883327-580ac500-5fcc-11eb-989c-6ff4a68acbe7.png)

SurferJeffAtGoogle: Approved!

On Tue, Jan 26, 2021 at 9:48 AM Tim Swast <notifications@github.com> wrote:

> @SurferJeffAtGoogle <https://github.com/SurferJeffAtGoogle> It looks like
> Bu Sun isn't in CODEOWNERS. I can't merge.
>
> [image: image]
> <https://user-images.githubusercontent.com/247555/105883327-580ac500-5fcc-11eb-989c-6ff4a68acbe7.png>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/googleapis/synthtool/pull/906#issuecomment-767713858>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACZ3TYTVWHE3M65DSNZ4IBDS3356PANCNFSM4WEKVUNA>
> .
>


Patch:
diff --git a/synthtool/gcp/templates/python_library/noxfile.py.j2 b/synthtool/gcp/templates/python_library/noxfile.py.j2
index ee187e7..e59cc3c 100644
--- a/synthtool/gcp/templates/python_library/noxfile.py.j2
+++ b/synthtool/gcp/templates/python_library/noxfile.py.j2
@@ -88,9 +88,21 @@ def default(session):
     session.install("asyncmock", "pytest-asyncio")
     {% endif %}
     session.install("mock", "pytest", "pytest-cov", {% for d in unit_test_external_dependencies %}"{{d}}"{% if not loop.last %},{% endif %}{% endfor %})
-    session.install("-e", ".")
-    {% for dependency in unit_test_local_dependencies %}session.install("-e", "{{dependency}}"){% endfor %}
+    {% for dependency in unit_test_local_dependencies %}session.install("-e", "{{dependency}}")
+    {% endfor %}
     {% for dependency in unit_test_dependencies %}session.install("-e", "{{dependency}}"){% 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}")
+    {% elif unit_test_extras %}
+    session.install("-e", ".[{{",".join(unit_test_extras)}}]")
+    {% else %}
+    session.install("-e", ".")
+    {% endif %}
 
     # Run py.test against the unit tests.
     session.run(
@@ -139,10 +151,21 @@ def system(session):
     session.install("mock", "pytest", "google-cloud-testutils", {% for d in system_test_external_dependencies %}"{{d}}"{% if not loop.last %},{% endif %}{% endfor %})
 
     {%- if system_test_local_dependencies %}
-    session.install("-e", {% for d in system_test_local_dependencies %}"{{d}}"{% if not loop.last %},{% endif %}{% endfor %})
+    {% for dependency in system_test_local_dependencies %}session.install("-e", "{{dependency}}")
+    {% 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}")
+    {% elif system_test_extras %}
+    session.install("-e", ".[{{",".join(system_test_extras)}}]")
+    {% else %}
     session.install("-e", ".")
-
+    {% endif %}
 
     # Run py.test against the system tests.
     if system_test_exists:


Test patch:
diff --git a/tests/test_python_library.py b/tests/test_python_library.py
index 9a263e3..a7a2236 100644
--- a/tests/test_python_library.py
+++ b/tests/test_python_library.py
@@ -15,7 +15,89 @@
 import os
 from pathlib import Path
 
+import pytest
+
 from synthtool import gcp
+from synthtool.sources import templates
+
+
+PYTHON_LIBRARY = Path(__file__).parent.parent / "synthtool/gcp/templates/python_library"
+
+
+@pytest.mark.parametrize(
+    ["template_kwargs", "expected_text"],
+    [
+        ({}, ["import nox", 'session.install("-e", ".")']),
+        (
+            {"unit_test_local_dependencies": ["../testutils", "../unitutils"]},
+            [
+                'session.install("-e", "../testutils")',
+                'session.install("-e", "../unitutils")',
+            ],
+        ),
+        (
+            {"system_test_local_dependencies": ["../testutils", "../sysutils"]},
+            [
+                'session.install("-e", "../testutils")',
+                'session.install("-e", "../sysutils")',
+            ],
+        ),
+        (
+            {"unit_test_extras": ["abc", "def"]},
+            ['session.install("-e", ".[abc,def]")'],
+        ),
+        (
+            {"system_test_extras": ["abc", "def"]},
+            ['session.install("-e", ".[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}")',
+            ],
+        ),
+        (
+            {"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}")',
+            ],
+        ),
+        (
+            {
+                "unit_test_extras": ["tuv", "wxyz"],
+                "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}")',
+            ],
+        ),
+        (
+            {
+                "system_test_extras": ["tuv", "wxyz"],
+                "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}")',
+            ],
+        ),
+    ],
+)
+def test_library_noxfile(template_kwargs, expected_text):
+    t = templates.Templates(PYTHON_LIBRARY)
+    result = t.render("noxfile.py.j2", **template_kwargs,).read_text()
+    # Validate Python syntax.
+    result_code = compile(result, "noxfile.py", "exec")
+    assert result_code is not None
+    for expected in expected_text:
+        assert expected in result
 
 
 def test_python_library():