From d358a2e6bb7c952f086b4ae59cb2e6a26d1ea482 Mon Sep 17 00:00:00 2001 From: Christoph Kuhnke Date: Thu, 27 Jun 2024 09:06:37 +0200 Subject: [PATCH] #295: Made notebook-tests mandatory for merge (#296) * #295: Made notebook-tests mandatory for merge * Updated GitHub workflow files * Moved shell check to check_ci.yaml * Removed hyperlinks checking from regular ci build * Sorted notebook files before running notebook tests * Added log message for executing each single notebook test file * #193: Ignored warnings in notebook tests * Moved logging options to pytest.ini for notebook tests * #297: Reduced log level for transitive libraries in notebook tests * Reduce logging in all notebooks Co-authored-by: Torsten Kilias --- .../workflows/check_documentation_links.yaml | 23 -------- .github/workflows/check_version.yaml | 21 -------- .github/workflows/{check_ci.yaml => ci.yaml} | 53 +++++++++++++------ ...ck_notebook_links.yaml => hyperlinks.yaml} | 22 +++++--- .github/workflows/shellcheck.yaml | 15 ------ doc/changes/changes_2.1.0.md | 5 +- doc/developer_guide/ci.md | 12 ++--- .../test_create_dss_docker_image.py | 1 + .../test_notebooks_in_dss_docker_image.py | 34 +++++------- test/notebooks/disabled_nbtest_cloud.py | 8 ++- test/notebooks/nbtest_itde.py | 2 + test/notebooks/nbtest_sagemaker.py | 18 ++++--- test/notebooks/nbtest_sklearn.py | 12 +++-- test/notebooks/nbtest_transformers.py | 14 +++-- test/notebooks/notebook_test_utils.py | 28 +++++++++- test/notebooks/pytest.ini | 10 +++- 16 files changed, 148 insertions(+), 130 deletions(-) delete mode 100644 .github/workflows/check_documentation_links.yaml delete mode 100644 .github/workflows/check_version.yaml rename .github/workflows/{check_ci.yaml => ci.yaml} (78%) rename .github/workflows/{check_notebook_links.yaml => hyperlinks.yaml} (53%) delete mode 100644 .github/workflows/shellcheck.yaml diff --git a/.github/workflows/check_documentation_links.yaml b/.github/workflows/check_documentation_links.yaml deleted file mode 100644 index 356cc4a7..00000000 --- a/.github/workflows/check_documentation_links.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: Check Documentation Links - -on: - push: - branches-ignore: - - "main" - -jobs: - check_documentation_links: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Link Checker - id: lychee - uses: lycheeverse/lychee-action@v1.9.0 - with: - fail: true - args: --verbose --no-progress 'doc/**/*.md' 'README.md' - diff --git a/.github/workflows/check_version.yaml b/.github/workflows/check_version.yaml deleted file mode 100644 index d0b0ba3e..00000000 --- a/.github/workflows/check_version.yaml +++ /dev/null @@ -1,21 +0,0 @@ -name: Check if versions are consistent - -on: - push: - branches-ignore: - - "main" - -jobs: - check-version-numbers: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Setup Python & Poetry Environment - uses: exasol/python-toolbox/.github/actions/python-environment@0.13.0 - with: - python-version: "3.10" - poetry-version: '1.8.2' - - name: Check Release - run: poetry run python3 -u "./scripts/build/check_release.py" diff --git a/.github/workflows/check_ci.yaml b/.github/workflows/ci.yaml similarity index 78% rename from .github/workflows/check_ci.yaml rename to .github/workflows/ci.yaml index b81d34cc..b48c8a2e 100644 --- a/.github/workflows/check_ci.yaml +++ b/.github/workflows/ci.yaml @@ -1,12 +1,11 @@ -name: CI Build +name: CI on: - push: - branches-ignore: - - "main" + pull_request: jobs: - run_unit_tests: + run-unit-tests: + name: Unit Tests runs-on: ubuntu-latest steps: @@ -14,11 +13,17 @@ jobs: with: fetch-depth: 0 + - name: Run shellcheck + run: ./scripts/build/shellcheck.sh + - name: Setup Python & Poetry Environment uses: exasol/python-toolbox/.github/actions/python-environment@0.13.0 with: python-version: "3.10" - poetry-version: '1.8.2' + poetry-version: "1.8.2" + + - name: Check Version Number + run: poetry run python3 -u "./scripts/build/check_release.py" - name: Run Unit Tests run: > @@ -28,14 +33,14 @@ jobs: --override-ini=log_cli_level=INFO test/unit - run_integration_tests: + run-integration-tests: name: Integration Tests environment: AWS_CI_TESTS runs-on: ubuntu-latest - needs: run_unit_tests + needs: run-unit-tests steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 @@ -61,8 +66,7 @@ jobs: uses: exasol/python-toolbox/.github/actions/python-environment@0.13.0 with: python-version: "3.10" - poetry-version: '1.8.2' - + poetry-version: "1.8.2" - name: Run Integration Tests run: > @@ -76,15 +80,23 @@ jobs: AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_ACCESS_KEY_SECRET }} AWS_DEFAULT_REGION: ${{ secrets.AWS_REGION }} - run_notebook_tests: + approval-for-notebook-tests: + name: Run Jupyter Notebook Tests? + runs-on: ubuntu-latest + steps: + - name: Detect Running Notebook Tests + run: true + environment: + approve-test-execution + + run-notebook-tests: name: Jupyter Notebook Tests - if: "contains(github.event.head_commit.message, '[run-notebook-tests]')" environment: AWS_SAGEMAKER runs-on: ubuntu-latest - needs: run_unit_tests + needs: approval-for-notebook-tests steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 @@ -110,8 +122,7 @@ jobs: uses: exasol/python-toolbox/.github/actions/python-environment@0.13.0 with: python-version: "3.10" - poetry-version: '1.8.2' - + poetry-version: "1.8.2" - name: Run notebook tests run: > @@ -127,3 +138,11 @@ jobs: SAAS_HOST: ${{ secrets.INTEGRATION_TEAM_SAAS_STAGING_HOST }} SAAS_ACCOUNT_ID: ${{ secrets.INTEGRATION_TEAM_SAAS_STAGING_ACCOUNT_ID }} SAAS_PAT: ${{ secrets.INTEGRATION_TEAM_SAAS_STAGING_PAT }} + + gate-2: + name: Gate 2 - Allow Merge + runs-on: ubuntu-latest + needs: [ run-unit-tests, run-integration-tests, run-notebook-tests ] + steps: + - name: Branch Protection + run: true diff --git a/.github/workflows/check_notebook_links.yaml b/.github/workflows/hyperlinks.yaml similarity index 53% rename from .github/workflows/check_notebook_links.yaml rename to .github/workflows/hyperlinks.yaml index 3282ebb8..d1c7b5ec 100644 --- a/.github/workflows/check_notebook_links.yaml +++ b/.github/workflows/hyperlinks.yaml @@ -1,25 +1,31 @@ -name: Check Notebook Links +name: Check Hyperlinks on: - push: - branches-ignore: - - "main" + pull_request: jobs: - check_notebook_links: + check-links: + name: Find Broken Links in Documentation and Jupyter Notebooks runs-on: ubuntu-latest - steps: + - uses: actions/checkout@v4 with: fetch-depth: 0 + - name: Setup Python & Poetry Environment uses: exasol/python-toolbox/.github/actions/python-environment@0.13.0 with: python-version: "3.10" poetry-version: '1.8.2' - - name: Run build ai-lab tests + - name: Check Hyperlinks in Documentation + id: lychee + uses: lycheeverse/lychee-action@v1.9.0 + with: + fail: true + args: --verbose --no-progress 'doc/**/*.md' 'README.md' + + - name: Check Hyperlinks in Jupyter Notebooks run: > poetry run pytest --check-links exasol/ds/sandbox/runtime/ansible/roles/jupyter/files/notebook/ - diff --git a/.github/workflows/shellcheck.yaml b/.github/workflows/shellcheck.yaml deleted file mode 100644 index a29939e5..00000000 --- a/.github/workflows/shellcheck.yaml +++ /dev/null @@ -1,15 +0,0 @@ -name: Check bash scripts - -on: - push: - branches: - - main - pull_request: - -jobs: - shellcheck: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: Run shellcheck - run: ./scripts/build/shellcheck.sh diff --git a/doc/changes/changes_2.1.0.md b/doc/changes/changes_2.1.0.md index 8418e725..ff7ebcd2 100644 --- a/doc/changes/changes_2.1.0.md +++ b/doc/changes/changes_2.1.0.md @@ -23,7 +23,7 @@ Version: 2.1.0 ## Bug Fixes -* #289 Fixed UI in notebooks +n/a ## Documentation @@ -35,3 +35,6 @@ Version: 2.1.0 * #267: Switched CodeBuildWaiter to use tenacity * #276: Started using the new ITDE Manager interface in the notebook-connector 0.2.9 * #282: Updated python version to Python 3.10 +* #295: Made notebook-tests mandatory for merge +* #193: Ignored warnings in notebook tests +* #297: Reduced log level for transitive libraries in notebook tests diff --git a/doc/developer_guide/ci.md b/doc/developer_guide/ci.md index 4c753814..948dc3fd 100644 --- a/doc/developer_guide/ci.md +++ b/doc/developer_guide/ci.md @@ -3,20 +3,16 @@ The project has two types of CI tests: * Unit tests and integration tests which run in a Github workflow * Special integration tests verifying the content of the Jupyter notebook files -* A system test which runs on a AWS Codebuild +* A system test which runs in a AWS Codebuild All these tests need to pass before the approval of a Github PR. The Github workflow will run on each push to a branch in the Github repository. -However, the notebook tests and the AWS Codebuild will only run after you push a commit containing a special string in the commit message, see the following sections. +However, the notebook tests and the AWS Codebuild will only run under specific conditions, e.g. manual approval or push a commit containing a special string in the commit message, see the following sections. -### Executing Notebook Tests +### Executing Jupyter Notebook Tests -Use the following git commands to execute the notebook tests: - -```shell -git commit -m "[run-notebook-tests]" --allow-empty && git push -``` +The regular CI build will ask for confirmation (aka. "review") before executing these tests, see [ETAJ developer guide](https://github.com/exasol/exasol-test-setup-abstraction-java/blob/main/doc/developer_guide/developer_guide.md#ci-build) for details. ### Executing AWS CodeBuild diff --git a/test/integration/test_create_dss_docker_image.py b/test/integration/test_create_dss_docker_image.py index 3ce7445d..947ed9c1 100644 --- a/test/integration/test_create_dss_docker_image.py +++ b/test/integration/test_create_dss_docker_image.py @@ -299,6 +299,7 @@ def user_and_group(ls_line: str) -> str: 'bind': notebooks_folder, 'mode': 'rw', }, }, ) as container: + wait_for(container, "entrypoint.py: Did chown -R") testees = [tmp_path, child, sub, grand_child] command = ls_command(str(tmp_path), notebooks_folder, testees) output = assert_exec_run(container, command, user=JUPYTER_USER) diff --git a/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py b/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py index bd4b97e0..09e058c7 100644 --- a/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py +++ b/test/notebook_test_runner/test_notebooks_in_dss_docker_image.py @@ -67,46 +67,38 @@ def notebook_test_container(request, notebook_test_image): @pytest.fixture() def notebook_test_container_with_log(notebook_test_container): - wait_for_socket_access(notebook_test_container) - logs = notebook_test_container.logs().decode("utf-8").strip() + container = notebook_test_container + wait_for_socket_access(container) + logs = container.logs().decode("utf-8").strip() print(f"Container Logs: {logs or '(empty)'}", flush=True) - yield notebook_test_container - - -def ignored_warnings(): - warnings = { - "DeprecationWarning": [ - "Jupyter is migrating its paths to use standard platformdirs", - "pkg_resources is deprecated as an API", - "Deprecated call to \\`pkg_resources.declare_namespace", - ] - } - args = "" - for category, messages in warnings.items(): - for m in messages: - args += f' -W "ignore:{m}:{category}"' - return args + yield container @pytest.mark.parametrize( "notebook_test_file", [ python_file.name - for python_file in TEST_RESOURCE_PATH.glob("nbtest_*.py") + for python_file in sorted(TEST_RESOURCE_PATH.glob("nbtest_*.py")) if python_file.is_file() ] ) def test_notebook(notebook_test_container_with_log, notebook_test_file): + _logger.info(f"Running notebook tests for {notebook_test_file}") container = notebook_test_container_with_log command_echo_virtual_env = 'bash -c "echo $VIRTUAL_ENV"' virtual_env = exec_command(command_echo_virtual_env, container) command_run_test = ( f"{virtual_env}/bin/python" f" -m pytest --setup-show -s {notebook_test_file}" - f"{ignored_warnings()}" ) environ = os.environ.copy() environ["NBTEST_ACTIVE"] = "TRUE" nbtest_environ = {key: value for key, value in environ.items() if ( key.startswith("NBTEST_") or key.startswith("SAAS_"))} - exec_command(command_run_test, container, print_output=True, environment=nbtest_environ, user="jupyter") + exec_command( + command_run_test, + container, + print_output=True, + environment=nbtest_environ, + user="jupyter", + ) diff --git a/test/notebooks/disabled_nbtest_cloud.py b/test/notebooks/disabled_nbtest_cloud.py index f37dcf70..fcf564a4 100644 --- a/test/notebooks/disabled_nbtest_cloud.py +++ b/test/notebooks/disabled_nbtest_cloud.py @@ -1,5 +1,11 @@ import os -from notebook_test_utils import (access_to_temp_secret_store, notebook_runner) +from notebook_test_utils import ( + access_to_temp_secret_store, + notebook_runner, + set_log_level_for_libraries, +) + +set_log_level_for_libraries() def test_cloud_notebook(notebook_runner) -> None: diff --git a/test/notebooks/nbtest_itde.py b/test/notebooks/nbtest_itde.py index 4d45222c..cd539613 100644 --- a/test/notebooks/nbtest_itde.py +++ b/test/notebooks/nbtest_itde.py @@ -4,7 +4,9 @@ take_itde_down ) from exasol.nb_connector.connections import open_pyexasol_connection +from notebook_test_utils import set_log_level_for_libraries +set_log_level_for_libraries() def test_itde(tmp_path): store_path = tmp_path / 'tmp_config.sqlite' diff --git a/test/notebooks/nbtest_sagemaker.py b/test/notebooks/nbtest_sagemaker.py index f1129dfc..837ebeb3 100644 --- a/test/notebooks/nbtest_sagemaker.py +++ b/test/notebooks/nbtest_sagemaker.py @@ -9,10 +9,16 @@ from exasol.nb_connector.secret_store import Secrets from exasol.nb_connector.ai_lab_config import AILabConfig as CKey, StorageBackend -from notebook_test_utils import (access_to_temp_secret_store, - access_to_temp_saas_secret_store, - run_notebook, - uploading_hack) +from notebook_test_utils import ( + access_to_temp_secret_store, + access_to_temp_saas_secret_store, + run_notebook, + uploading_hack, + set_log_level_for_libraries, +) + + +set_log_level_for_libraries() def _create_aws_s3_bucket() -> str: @@ -170,7 +176,7 @@ def continuous_job_polling(): time.sleep(30) query_result = conn.execute(sql) job_status = query_result.fetchall()[0][0] - + continuous_job_polling() """) ) @@ -186,7 +192,7 @@ def test_sagemaker(access_to_temp_secret_store, uploading_hack): bucket_name = _create_aws_s3_bucket() role_name = _create_sagemaker_role_with_policy() _store_aws_credentials(store_path, store_password, bucket_name, role_name) - + current_dir = os.getcwd() try: run_notebook('main_config.ipynb', store_file, store_password) diff --git a/test/notebooks/nbtest_sklearn.py b/test/notebooks/nbtest_sklearn.py index fb75396d..c2ce925b 100644 --- a/test/notebooks/nbtest_sklearn.py +++ b/test/notebooks/nbtest_sklearn.py @@ -2,9 +2,15 @@ import pytest from exasol.nb_connector.ai_lab_config import StorageBackend -from notebook_test_utils import (access_to_temp_secret_store, - access_to_temp_saas_secret_store, - notebook_runner) +from notebook_test_utils import ( + access_to_temp_secret_store, + access_to_temp_saas_secret_store, + notebook_runner, + set_log_level_for_libraries, +) + + +set_log_level_for_libraries() @pytest.mark.parametrize('notebook_runner', [StorageBackend.onprem, StorageBackend.saas], indirect=True) diff --git a/test/notebooks/nbtest_transformers.py b/test/notebooks/nbtest_transformers.py index 141bc194..256a0adc 100644 --- a/test/notebooks/nbtest_transformers.py +++ b/test/notebooks/nbtest_transformers.py @@ -2,10 +2,16 @@ import textwrap import pytest -from notebook_test_utils import (access_to_temp_secret_store, - access_to_temp_saas_secret_store, - notebook_runner, - uploading_hack) +from notebook_test_utils import ( + access_to_temp_secret_store, + access_to_temp_saas_secret_store, + notebook_runner, + uploading_hack, + set_log_level_for_libraries, +) + + +set_log_level_for_libraries() @pytest.mark.parametrize( diff --git a/test/notebooks/notebook_test_utils.py b/test/notebooks/notebook_test_utils.py index 573a8d30..9a01a550 100644 --- a/test/notebooks/notebook_test_utils.py +++ b/test/notebooks/notebook_test_utils.py @@ -6,7 +6,9 @@ import textwrap from contextlib import contextmanager, ExitStack from datetime import timedelta +import logging import os +from inspect import cleandoc import pytest import nbformat @@ -25,6 +27,7 @@ timestamp_name, ) +LOG = logging.getLogger(__name__) def _env(var: str) -> str: result = os.environ.get(var) @@ -113,6 +116,29 @@ def init_notebook_test(): nb_client.execute() + +# ~/git/ai-lab/test/notebooks/notebook_test_utils.py +def set_log_level_for_libraries(level=logging.WARNING): + modules = cleandoc( + """ + traitlets + luigi-interface + luigi-interface.PrepareDockerNetworkForTestEnvironment + luigi-interface.SpawnTestDockerDatabase + luigi-interface.SpawnTestEnvironmentWithDockerDB + luigi-interface.WaitForTestDockerDatabase + httpx + """ + ).split() + LOG.info( + f"Setting log level to '%s' for modules\n - %s", + logging.getLevelName(level), + "\n - ".join(modules), + ) + for m in modules: + logging.getLogger(m).setLevel(level) + + @contextmanager def access_to_temp_onprem_secret_store(tmp_path: Path) -> Tuple[Path, str]: """ @@ -232,7 +258,7 @@ def uploading_hack() -> Tuple[str, str]: def pause_notebook_execution(): import time time.sleep(20) - + pause_notebook_execution() """) ) diff --git a/test/notebooks/pytest.ini b/test/notebooks/pytest.ini index be8d0936..7c1493da 100644 --- a/test/notebooks/pytest.ini +++ b/test/notebooks/pytest.ini @@ -1,2 +1,10 @@ [pytest] -python_files = nbtest_*.py \ No newline at end of file +python_files = nbtest_*.py +filterwarnings = + ignore:Jupyter is migrating its paths to use standard platformdirs:DeprecationWarning + ignore:pkg_resources is deprecated as an API:DeprecationWarning + ignore:Deprecated call to \`pkg_resources.declare_namespace:DeprecationWarning + ignore::DeprecationWarning:luigi.*: + ignore::DeprecationWarning:pkg_resources.*: +log_cli = 1 +log_cli_level = INFO