From e70e65af8755d9e12f226b90fce5285cc4692400 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 10 May 2024 11:36:00 -0400 Subject: [PATCH] Mark flaky tests (#813) Backport #813 (cherry picked from commit b57890e3503928ced7dfd75de6985b348f0e7307) --- .github/scripts/integration-test-matrix.js | 87 --------- .github/workflows/integration.yml | 170 ++++++++++-------- .github/workflows/main.yml | 2 +- pytest.ini | 2 + tests/conftest.py | 11 ++ .../adapter/catalog_tests/test_get_catalog.py | 0 .../catalog_tests/test_relation_types.py | 1 + .../test_materialized_views.py | 6 +- tests/functional/adapter/test_basic.py | 12 +- tests/functional/adapter/test_persist_docs.py | 13 +- tox.ini | 1 + 11 files changed, 139 insertions(+), 166 deletions(-) delete mode 100644 .github/scripts/integration-test-matrix.js create mode 100644 tests/functional/adapter/catalog_tests/test_get_catalog.py diff --git a/.github/scripts/integration-test-matrix.js b/.github/scripts/integration-test-matrix.js deleted file mode 100644 index ccb6d949d..000000000 --- a/.github/scripts/integration-test-matrix.js +++ /dev/null @@ -1,87 +0,0 @@ -module.exports = ({ context }) => { - const defaultPythonVersion = "3.8"; - const supportedPythonVersions = ["3.8", "3.9", "3.10", "3.11"]; - const supportedAdapters = ["redshift"]; - - // if PR, generate matrix based on files changed and PR labels - if (context.eventName.includes("pull_request")) { - // `changes` is a list of adapter names that have related - // file changes in the PR - // ex: ['postgres', 'snowflake'] - const labels = context.payload.pull_request.labels.map(({ name }) => name); - console.log("labels", labels); - const testAllLabel = labels.includes("test all"); - const include = []; - - for (const adapter of supportedAdapters) { - for (const pythonVersion of supportedPythonVersions) { - if ( - pythonVersion === defaultPythonVersion || - labels.includes(`test python${pythonVersion}`) || - testAllLabel - ) { - // always run tests on ubuntu by default - include.push({ - os: "ubuntu-latest", - adapter, - "python-version": pythonVersion, - }); - - if (labels.includes("test windows") || testAllLabel) { - include.push({ - os: "windows-latest", - adapter, - "python-version": pythonVersion, - }); - } - - if (labels.includes("test macos") || testAllLabel) { - include.push({ - os: "macos-12", - adapter, - "python-version": pythonVersion, - }); - } - } - } - } - - console.log("matrix", { include }); - - return { - include, - }; - } - // if not PR, generate matrix of python version, adapter, and operating - // system to run integration tests on - - const include = []; - // run for all adapters and python versions on ubuntu - for (const adapter of supportedAdapters) { - for (const pythonVersion of supportedPythonVersions) { - include.push({ - os: 'ubuntu-latest', - adapter: adapter, - "python-version": pythonVersion, - }); - } - } - - // additionally include runs for all adapters, on macos and windows, - // but only for the default python version - for (const adapter of supportedAdapters) { - for (const operatingSystem of ["windows-latest", "macos-12"]) { - include.push({ - os: operatingSystem, - adapter: adapter, - "python-version": defaultPythonVersion, - }); - } - } - - console.log("matrix", { include }); - - return { - include, - }; -}; diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 3eec2b27f..5c0c9e73d 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -53,25 +53,49 @@ defaults: shell: bash jobs: - # generate test metadata about what files changed and the testing matrix to use - test-metadata: + test: + name: redshift / python ${{ matrix.python-version }} / ${{ matrix.os }} + # run if not a PR from a forked repository or has a label to mark as safe to test if: >- github.event_name != 'pull_request_target' || github.event.pull_request.head.repo.full_name == github.repository || contains(github.event.pull_request.labels.*.name, 'ok to test') - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} - outputs: - matrix: ${{ steps.generate-matrix.outputs.result }} + strategy: + fail-fast: false + max-parallel: 3 + matrix: + os: [ubuntu-22.04, macos12, windows-2022] + python-version: ["3.8"] + include: + - os: ubuntu-22.04 + python-version: "3.9" + - os: ubuntu-22.04 + python-version: "3.10" + - os: ubuntu-22.04 + python-version: "3.11" + + env: + TOXENV: integration-redshift + PYTEST_ADDOPTS: "-v --color=yes -n4 --csv integration_results.csv" + DBT_INVOCATION_ENV: github-actions + DD_CIVISIBILITY_AGENTLESS_ENABLED: true + DD_API_KEY: ${{ secrets.DATADOG_API_KEY }} + DD_SITE: datadoghq.com + DD_ENV: ci + DD_SERVICE: ${{ github.event.repository.name }} steps: - - name: Check out the repository (non-PR) + - name: Check out the repository if: github.event_name != 'pull_request_target' uses: actions/checkout@v3 with: persist-credentials: false + # explicity checkout the branch for the PR, + # this is necessary for the `pull_request_target` event - name: Check out the repository (PR) if: github.event_name == 'pull_request_target' uses: actions/checkout@v3 @@ -79,62 +103,79 @@ jobs: persist-credentials: false ref: ${{ github.event.pull_request.head.sha }} - - name: Check if relevant files changed - if: github.event_name == 'pull_request_target' - # https://github.com/marketplace/actions/paths-changes-filter - # For each filter, it sets output variable named by the filter to the text: - # 'true' - if any of changed files matches any of filter rules - # 'false' - if none of changed files matches any of filter rules - # also, returns: - # `changes` - JSON array with names of all filters matching any of the changed files - uses: dorny/paths-filter@v2 - id: get-changes + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 with: - token: ${{ secrets.GITHUB_TOKEN }} - filters: | - redshift: - - 'dbt/**' - - 'tests/**' - - 'dev-requirements.txt' - - - name: Generate integration test matrix - id: generate-matrix - uses: actions/github-script@v6 + python-version: ${{ matrix.python-version }} + + - name: Install python dependencies + run: | + python -m pip install --user --upgrade pip + python -m pip install tox + python -m pip --version + tox --version + + - name: Update dev_requirements.txt + if: inputs.dbt-core-branch != '' + run: | + pip install bumpversion + ./.github/scripts/update_dbt_core_branch.sh ${{ inputs.dbt-core-branch }} + + - name: Run tox (redshift) env: - CHANGES: ${{ steps.get-changes.outputs.changes }} + REDSHIFT_TEST_DBNAME: ${{ secrets.REDSHIFT_TEST_DBNAME }} + REDSHIFT_TEST_PASS: ${{ secrets.REDSHIFT_TEST_PASS }} + REDSHIFT_TEST_USER: ${{ secrets.REDSHIFT_TEST_USER }} + REDSHIFT_TEST_PORT: ${{ secrets.REDSHIFT_TEST_PORT }} + REDSHIFT_TEST_HOST: ${{ secrets.REDSHIFT_TEST_HOST }} + DBT_TEST_USER_1: dbt_test_user_1 + DBT_TEST_USER_2: dbt_test_user_2 + DBT_TEST_USER_3: dbt_test_user_3 + run: tox -- -m "not flaky" --ddtrace + + - uses: actions/upload-artifact@v3 + if: always() with: - script: | - const script = require('./.github/scripts/integration-test-matrix.js') - const matrix = script({ context }) - console.log(matrix) - return matrix - test: - name: ${{ matrix.adapter }} / python ${{ matrix.python-version }} / ${{ matrix.os }} + name: logs + path: ./logs + + - name: Get current date + if: always() + id: date + run: | + echo "date=$(date +'%Y-%m-%dT%H_%M_%S')" >> $GITHUB_OUTPUT #no colons allowed for artifacts + + - uses: actions/upload-artifact@v3 + if: always() + with: + name: integration_results_${{ matrix.python-version }}_${{ matrix.os }}_redshift-${{ steps.date.outputs.date }}.csv + path: integration_results.csv + + test-flaky: + name: redshift / python ${{ matrix.python-version }} / ubuntu-22.04 - flaky + + # run this after the norm integration tests to avoid collisions + needs: test # run if not a PR from a forked repository or has a label to mark as safe to test - # also checks that the matrix generated is not empty if: >- - needs.test-metadata.outputs.matrix && - fromJSON( needs.test-metadata.outputs.matrix ).include[0] && - ( - github.event_name != 'pull_request_target' || - github.event.pull_request.head.repo.full_name == github.repository || - contains(github.event.pull_request.labels.*.name, 'ok to test') - ) - runs-on: ${{ matrix.os }} - - needs: test-metadata + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository || + contains(github.event.pull_request.labels.*.name, 'ok to test') + runs-on: ubuntu-22.04 strategy: fail-fast: false - max-parallel: 3 - matrix: ${{ fromJSON(needs.test-metadata.outputs.matrix) }} + max-parallel: 1 + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11"] env: - TOXENV: integration-${{ matrix.adapter }} - PYTEST_ADDOPTS: "-v --color=yes -n4 --csv integration_results.csv" + TOXENV: integration-redshift + PYTEST_ADDOPTS: "-v --color=yes -n1 --csv integration_results.csv" DBT_INVOCATION_ENV: github-actions DD_CIVISIBILITY_AGENTLESS_ENABLED: true + DD_INSTRUMENTATION_TELEMETRY_ENABLED: false DD_API_KEY: ${{ secrets.DATADOG_API_KEY }} DD_SITE: datadoghq.com DD_ENV: ci @@ -175,38 +216,25 @@ jobs: ./.github/scripts/update_dbt_core_branch.sh ${{ inputs.dbt-core-branch }} - name: Run tox (redshift) - if: matrix.adapter == 'redshift' env: REDSHIFT_TEST_DBNAME: ${{ secrets.REDSHIFT_TEST_DBNAME }} REDSHIFT_TEST_PASS: ${{ secrets.REDSHIFT_TEST_PASS }} REDSHIFT_TEST_USER: ${{ secrets.REDSHIFT_TEST_USER }} REDSHIFT_TEST_PORT: ${{ secrets.REDSHIFT_TEST_PORT }} REDSHIFT_TEST_HOST: ${{ secrets.REDSHIFT_TEST_HOST }} + REDSHIFT_TEST_REGION: ${{ vars.REDSHIFT_TEST_REGION }} + REDSHIFT_TEST_CLUSTER_ID: ${{ vars.REDSHIFT_TEST_CLUSTER_ID }} + REDSHIFT_TEST_IAM_USER_PROFILE: ${{ vars.REDSHIFT_TEST_IAM_USER_PROFILE }} + REDSHIFT_TEST_IAM_USER_ACCESS_KEY_ID: ${{ vars.REDSHIFT_TEST_IAM_USER_ACCESS_KEY_ID }} + REDSHIFT_TEST_IAM_USER_SECRET_ACCESS_KEY: ${{ secrets.REDSHIFT_TEST_IAM_USER_SECRET_ACCESS_KEY }} + REDSHIFT_TEST_IAM_ROLE_PROFILE: ${{ vars.REDSHIFT_TEST_IAM_ROLE_PROFILE }} DBT_TEST_USER_1: dbt_test_user_1 DBT_TEST_USER_2: dbt_test_user_2 DBT_TEST_USER_3: dbt_test_user_3 - run: tox -- --ddtrace - - - uses: actions/upload-artifact@v3 - if: always() - with: - name: logs - path: ./logs - - - name: Get current date - if: always() - id: date - run: | - echo "date=$(date +'%Y-%m-%dT%H_%M_%S')" >> $GITHUB_OUTPUT #no colons allowed for artifacts - - - uses: actions/upload-artifact@v3 - if: always() - with: - name: integration_results_${{ matrix.python-version }}_${{ matrix.os }}_${{ matrix.adapter }}-${{ steps.date.outputs.date }}.csv - path: integration_results.csv + run: tox -- -m flaky --ddtrace require-label-comment: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: test @@ -229,7 +257,7 @@ jobs: check_for_duplicate_msg: true post-failure: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: test if: ${{ failure() }} diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e0e6fd20a..d6387ef30 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -173,7 +173,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-12, windows-latest] + os: [ubuntu-22.04, macos-12, windows-2022] python-version: ['3.8', '3.9', '3.10', '3.11'] steps: diff --git a/pytest.ini b/pytest.ini index b3d74bc14..8c290dc14 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,3 +7,5 @@ env_files = testpaths = tests/unit tests/functional +markers = + flaky: marks tests as flaky so they run one at a time (de-select with '-m "not flaky"') diff --git a/tests/conftest.py b/tests/conftest.py index 96f0d43e4..e1c9d6102 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,3 +20,14 @@ def dbt_profile_target(): "pass": os.getenv("REDSHIFT_TEST_PASS"), "dbname": os.getenv("REDSHIFT_TEST_DBNAME"), } + + +def pytest_sessionfinish(session, exitstatus): + """ + Configures pytest to treat a scenario with no tests as passing + + pytest returns a code 5 when it collects no tests in an effort to warn when tests are expected but not collected + We don't want this when running tox because some combinations of markers and test segments return nothing + """ + if exitstatus == 5: + session.exitstatus = 0 diff --git a/tests/functional/adapter/catalog_tests/test_get_catalog.py b/tests/functional/adapter/catalog_tests/test_get_catalog.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/functional/adapter/catalog_tests/test_relation_types.py b/tests/functional/adapter/catalog_tests/test_relation_types.py index 9b9156dec..657bf215b 100644 --- a/tests/functional/adapter/catalog_tests/test_relation_types.py +++ b/tests/functional/adapter/catalog_tests/test_relation_types.py @@ -24,6 +24,7 @@ def docs(self, project): run_dbt(["run"]) yield run_dbt(["docs", "generate"]) + @pytest.mark.flaky @pytest.mark.parametrize( "node_name,relation_type", [ diff --git a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py index 7bb5bcee6..f87eca9c5 100644 --- a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py +++ b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py @@ -178,6 +178,7 @@ def test_change_is_not_applied_via_alter(self, project, my_materialized_view): assert_message_in_logs(f"Applying ALTER to: {my_materialized_view}", logs, False) assert_message_in_logs(f"Applying REPLACE to: {my_materialized_view}", logs, False) + @pytest.mark.flaky def test_change_is_not_applied_via_replace(self, project, my_materialized_view): self.check_start_state(project, my_materialized_view) @@ -202,4 +203,7 @@ class TestRedshiftMaterializedViewChangesFail( RedshiftMaterializedViewChanges, MaterializedViewChangesFailMixin ): # Note: using retries doesn't work when we expect `dbt_run` to fail - pass + + @pytest.mark.flaky + def test_change_is_not_applied_via_replace(self, project, my_materialized_view): + super().test_change_is_not_applied_via_replace(project, my_materialized_view) diff --git a/tests/functional/adapter/test_basic.py b/tests/functional/adapter/test_basic.py index 64ab24e42..cd04bb1ba 100644 --- a/tests/functional/adapter/test_basic.py +++ b/tests/functional/adapter/test_basic.py @@ -38,7 +38,9 @@ # TODO: update these with test cases or remove them if not needed class TestSimpleMaterializationsRedshift(BaseSimpleMaterializations): - pass + @pytest.mark.flaky + def test_base(self, project): + super().test_base(project) class TestSingularTestsRedshift(BaseSingularTests): @@ -54,11 +56,15 @@ class TestEmptyRedshift(BaseEmpty): class TestEphemeralRedshift(BaseEphemeral): - pass + @pytest.mark.flaky + def test_ephemeral(self, project): + super().test_ephemeral(project) class TestIncrementalRedshift(BaseIncremental): - pass + @pytest.mark.flaky + def test_incremental(self, project): + super().test_incremental(project) class TestGenericTestsRedshift(BaseGenericTests): diff --git a/tests/functional/adapter/test_persist_docs.py b/tests/functional/adapter/test_persist_docs.py index 61b8bd5a6..6eeaf881c 100644 --- a/tests/functional/adapter/test_persist_docs.py +++ b/tests/functional/adapter/test_persist_docs.py @@ -12,15 +12,21 @@ class TestPersistDocs(BasePersistDocs): - pass + @pytest.mark.flaky + def test_has_comments_pglike(self, project): + super().test_has_comments_pglike(project) class TestPersistDocsColumnMissing(BasePersistDocsColumnMissing): - pass + @pytest.mark.flaky + def test_missing_column(self, project): + super().test_missing_column(project) class TestPersistDocsCommentOnQuotedColumn(BasePersistDocsCommentOnQuotedColumn): - pass + @pytest.mark.flaky + def test_quoted_column_comments(self, run_has_comments): + super().test_quoted_column_comments(run_has_comments) class TestPersistDocsLateBinding(BasePersistDocsBase): @@ -40,6 +46,7 @@ def project_config_update(self): } } + @pytest.mark.flaky def test_comment_on_late_binding_view(self, project): run_dbt() run_dbt(["docs", "generate"]) diff --git a/tox.ini b/tox.ini index c490fed9a..462bc8f07 100644 --- a/tox.ini +++ b/tox.ini @@ -21,6 +21,7 @@ passenv = REDSHIFT_TEST_* PYTEST_ADDOPTS DD_CIVISIBILITY_AGENTLESS_ENABLED + DD_INSTRUMENTATION_TELEMETRY_ENABLED DD_API_KEY DD_SITE DD_ENV