Fixes transform.ipynb and adds notebook testing (#3497)

Fixes a missing import statement from transform.ipynb, also, adds notebook tests.

The notebook tests run in parallel and each of them in their own virtual env, ensuring that the `pip install` statements work well. This mirrors how the tests are run on devsite pipelines as well. Hopefully we can catch errors faster this way!

Follow-up potentials:
- run the notebooks in non-isolated virtualenv _as well_ - so that we test the "running jupyter notebook from the repo" use case too.
- parallelize on the Github Action job level the notebooks - it is already taking 11 mins :(

I tested treebeard (#3504) github action, and finally settled on rolling my own driver with an executor. For executor I chose papermill as it simply worked just fine with xdist. Though probably I could use treebeard (?) - not sure what the benefit would be for individual notebook execution. `jupyter nbconvert --execute` didn't seem to execute `pip installs`? But I haven't gone too deep in that direction.

Fixes #2711.

.github/workflows/ci.yml

@@ -207,3 +207,18 @@ jobs:
           pip install -r dev_tools/conf/pip-list-dev-tools.txt
       - name: Pytest check
         run: check/pytest --ignore=cirq/contrib --benchmark-skip
+  notebooks:
+    name: Notebook Tests
+    runs-on: ubuntu-16.04
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+      - uses: actions/setup-python@v1
+        with:
+          python-version: '3.7'
+          architecture: 'x64'
+      - name: Install requirements
+        run: pip install pytest pytest-xdist requests virtualenv filelock
+      - name: Notebook tests
+        run: check/pytest -n auto -m slow dev_tools/notebook_test.py

dev_tools/conf/mypy.ini

@@ -5,7 +5,7 @@ follow_imports = silent
 ignore_missing_imports = true
 
 # 3rd-party libs for which we don't have stubs
-[mypy-apiclient.*,freezegun.*,matplotlib.*,mpl_toolkits,multiprocessing.dummy,numpy.*,oauth2client.*,pandas.*,pytest.*,scipy.*,sortedcontainers.*,setuptools.*,pylatex.*,networkx.*,qiskit.*,pypandoc.*,ply.*,_pytest.*,google.api.*,google.api_core.*,grpc.*,google.oauth2.*,google.protobuf.text_format.*,quimb.*,pyquil.*,google.cloud.*]
+[mypy-apiclient.*,freezegun.*,matplotlib.*,mpl_toolkits,multiprocessing.dummy,numpy.*,oauth2client.*,pandas.*,pytest.*,scipy.*,sortedcontainers.*,setuptools.*,pylatex.*,networkx.*,qiskit.*,pypandoc.*,ply.*,_pytest.*,google.api.*,google.api_core.*,grpc.*,google.oauth2.*,google.protobuf.text_format.*,quimb.*,pyquil.*,google.cloud.*,filelock.*]
 follow_imports = silent
 ignore_missing_imports = true
 

dev_tools/conf/pip-install-minimal-for-pytest-changed-files.sh

@@ -25,6 +25,7 @@ pip install -r requirements.txt
 
 # Install pytest related dev requirements.
 cat dev_tools/conf/pip-list-dev-tools.txt | grep pytest | xargs pip install
+cat dev_tools/conf/pip-list-dev-tools.txt | grep filelock | xargs pip install
 
 # Install contrib requirements only if needed.
 changed=$(git diff --name-only origin/master | grep "cirq/contrib" || true)

dev_tools/conf/pip-list-dev-tools.txt

@@ -6,6 +6,7 @@ pytest-asyncio~=0.12.0
 pytest-cov~=2.5.0
 pytest-benchmark~=3.2.0
 yapf~=0.27.0
+filelock
 
 # For generating protobufs
 

dev_tools/conftest.py

@@ -0,0 +1,31 @@
+# Copyright 2020 The Cirq Developers
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import pytest
+
+
+def pytest_configure(config):
+    config.addinivalue_line("markers", "slow: mark tests as slow")
+
+
+def pytest_collection_modifyitems(config, items):
+    keywordexpr = config.option.keyword
+    markexpr = config.option.markexpr
+    if keywordexpr or markexpr:
+        return  # let pytest handle this
+
+    skip_slow_marker = pytest.mark.skip(reason='slow marker not selected')
+    for item in items:
+        if 'slow' in item.keywords:
+            item.add_marker(skip_slow_marker)

dev_tools/notebook_test.py

@@ -0,0 +1,114 @@
+# Copyright 2020 The Cirq Developers
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+import functools
+import glob
+import os
+import sys
+
+import pytest
+from filelock import FileLock
+
+from dev_tools import shell_tools
+from dev_tools.env_tools import create_virtual_env
+
+SKIP_NOTEBOOKS = [
+    # skipping vendor notebooks as we don't have auth sorted out
+    "**/google/*.ipynb",
+    "**/pasqal/*.ipynb",
+    "**/aqt/*.ipynb",
+
+    # skipping quantum volume notebooks as they have issues
+    # see https://github.com/quantumlib/Cirq/issues/3501
+    "examples/advanced/*.ipynb",
+
+    # skipping fidelity estimation due to
+    # https://github.com/quantumlib/Cirq/issues/3502
+    "examples/*fidelity*"
+]
+
+
+def _tested_notebooks():
+    all_notebooks = set(glob.glob("**/*.ipynb", recursive=True))
+    skipped_notebooks = functools.reduce(
+        lambda a, b: a.union(b),
+        list(set(glob.glob(g, recursive=True)) for g in SKIP_NOTEBOOKS))
+
+    # sorted is important otherwise pytest-xdist will complain that
+    # the workers have differnent parametrization:
+    # https://github.com/pytest-dev/pytest-xdist/issues/432
+    return sorted(
+        os.path.abspath(n) for n in all_notebooks.difference(skipped_notebooks))
+
+
+TESTED_NOTEBOOKS = _tested_notebooks()
+
+PACKAGES = [
+    # for running the notebooks
+    "papermill",
+    "jupyter",
+    "virtualenv-clone",
+    # assumed to be part of colab
+    "seaborn",
+]
+
+
+@pytest.mark.slow
+@pytest.fixture(scope="session")
+def base_env(tmp_path_factory, worker_id):
+    # get the temp directory shared by all workers
+    root_tmp_dir = tmp_path_factory.getbasetemp().parent.parent
+    proto_dir = root_tmp_dir / "proto_dir"
+    with FileLock(str(proto_dir) + ".lock"):
+        if proto_dir.is_dir():
+            print(f"{worker_id} returning as {proto_dir} is a dir!")
+            print(f"If all the notebooks are failing, the test framework might "
+                  f"have left this directory around. Try 'rm -rf {proto_dir}'")
+        else:
+            print(f"{worker_id} creating stuff...")
+            create_base_env(proto_dir)
+
+    return root_tmp_dir, proto_dir
+
+
+def create_base_env(proto_dir):
+    create_virtual_env(str(proto_dir), [], sys.executable, True)
+    pip_path = str(proto_dir / "bin" / "pip")
+    shell_tools.run_cmd(pip_path, "install", *PACKAGES)
+
+
+@pytest.mark.slow
+@pytest.mark.parametrize("notebook_path", TESTED_NOTEBOOKS)
+def test_notebooks(notebook_path, base_env):
+    """Ensures testing the notebooks in isolated virtual environments."""
+    tmpdir, proto_dir = base_env
+
+    notebook_file = os.path.basename(notebook_path)
+    dir_name = notebook_file.rstrip(".ipynb")
+
+    notebook_env = os.path.join(tmpdir, f"{dir_name}")
+    cmd = f"""
+{proto_dir}/bin/virtualenv-clone {proto_dir} {notebook_env}
+cd {notebook_env}
+. ./bin/activate
+papermill {notebook_path}"""
+    stdout, stderr, status = shell_tools.run_shell(cmd=cmd,
+                                                   log_run_to_stderr=False,
+                                                   raise_on_fail=False,
+                                                   out=shell_tools.TeeCapture(),
+                                                   err=shell_tools.TeeCapture())
+
+    if status != 0:
+        print(stdout)
+        print(stderr)
+        pytest.fail(f"Notebook failure: {notebook_file}")

docs/transform.ipynb

@@ -99,6 +99,7 @@
    },
    "outputs": [],
    "source": [
+    "import cirq\n",
     "c=cirq.Circuit()\n",
     "c.append(cirq.Moment([]))\n",
     "c.append(cirq.Moment([cirq.X(cirq.GridQubit(1,1))]))\n",