From 575fe09642d2eaa35b74fd05d2253ec614dcf8d0 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) * mark flaky tests using pytest.mark * split integration test runs by flaky and non-flaky * fix the posargs syntax for tox * force flaky tests to run in series, fix python version getting truncated * revert the integration test job name to match the existing names * separate flaky tests as a separate matrix of jobs * recombine the integration tests * allow integration tests to run fully in parallel for non-flaky tests * remove windows from integration test matrix since we weren't originally testing on windows anyway * add windows back in * register the custom marker with pytest * revert the combination of integration tests * configure a pytest return of 5 to be 0 * fix test order to avoid unnecessary change * mark more flaky tests * mark more flaky tests * pin windows images to 2019 to avoid datadog traceport failures * pinning windows images to 2019 didn't solve datadog issue, updating pin to the most recent version * turn off telemetry warnings (and telemetry) * mark more tests as flaky * mark more tests as flaky * incorporate feedback * incorporate feedback (cherry picked from commit b57890e3503928ced7dfd75de6985b348f0e7307) --- .github/scripts/integration-test-matrix.js | 87 --------- .github/workflows/integration.yml | 172 ++++++++++-------- .github/workflows/main.yml | 2 +- pytest.ini | 2 + tests/conftest.py | 11 ++ .../adapter/catalog_tests/test_get_catalog.py | 2 + .../catalog_tests/test_relation_types.py | 1 + .../test_materialized_views.py | 15 +- tests/functional/adapter/test_basic.py | 12 +- tests/functional/adapter/test_persist_docs.py | 13 +- tox.ini | 1 + 11 files changed, 151 insertions(+), 167 deletions(-) delete mode 100644 .github/scripts/integration-test-matrix.js 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 ad29fef1e..0620392ba 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -53,85 +53,32 @@ 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 - - outputs: - matrix: ${{ steps.generate-matrix.outputs.result }} - - steps: - - name: Check out the repository (non-PR) - if: github.event_name != 'pull_request_target' - uses: actions/checkout@v3 - with: - persist-credentials: false - - - name: Check out the repository (PR) - if: github.event_name == 'pull_request_target' - uses: actions/checkout@v3 - with: - 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 - 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 - env: - CHANGES: ${{ steps.get-changes.outputs.changes }} - 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 }} - - # 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 - strategy: fail-fast: false max-parallel: 3 - matrix: ${{ fromJSON(needs.test-metadata.outputs.matrix) }} + 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-${{ matrix.adapter }} + TOXENV: integration-redshift PYTEST_ADDOPTS: "-v --color=yes -n4 --csv integration_results.csv" DBT_INVOCATION_ENV: github-actions DD_CIVISIBILITY_AGENTLESS_ENABLED: true @@ -202,7 +149,6 @@ jobs: AWS_REGION: ${{ vars.REDSHIFT_TEST_REGION }} - name: Run tox (redshift) - if: matrix.adapter == 'redshift' env: REDSHIFT_TEST_DBNAME: ${{ secrets.REDSHIFT_TEST_DBNAME }} REDSHIFT_TEST_PASS: ${{ secrets.REDSHIFT_TEST_PASS }} @@ -218,7 +164,7 @@ jobs: 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 + run: tox -- -m "not flaky" --ddtrace - uses: actions/upload-artifact@v3 if: always() @@ -235,11 +181,93 @@ jobs: - uses: actions/upload-artifact@v3 if: always() with: - name: integration_results_${{ matrix.python-version }}_${{ matrix.os }}_${{ matrix.adapter }}-${{ steps.date.outputs.date }}.csv + 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 + 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-22.04 + + strategy: + fail-fast: false + max-parallel: 1 + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11"] + + env: + 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 + DD_SERVICE: ${{ github.event.repository.name }} + + steps: + - 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 + with: + persist-credentials: false + ref: ${{ github.event.pull_request.head.sha }} + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + 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: + 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 -- -m flaky --ddtrace + require-label-comment: - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 needs: test @@ -262,7 +290,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 d8c126301..c49b57385 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 index 519c806c4..e0b512896 100644 --- a/tests/functional/adapter/catalog_tests/test_get_catalog.py +++ b/tests/functional/adapter/catalog_tests/test_get_catalog.py @@ -97,6 +97,7 @@ def my_information_schema(self, adapter, my_schema): identifier="INFORMATION_SCHEMA", ).information_schema() + @pytest.mark.flaky def test_get_one_catalog_by_relations( self, adapter, @@ -120,6 +121,7 @@ def test_get_one_catalog_by_relations( # note the underlying table is missing as it's not in `my_relations` assert len(catalog) == 12 + @pytest.mark.flaky def test_get_one_catalog_by_schemas( self, adapter, 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 64f697e77..f201196b4 100644 --- a/tests/functional/adapter/materialized_view_tests/test_materialized_views.py +++ b/tests/functional/adapter/materialized_view_tests/test_materialized_views.py @@ -75,6 +75,14 @@ def test_materialized_view_create_idempotent(self, project, my_materialized_view ) assert self.query_relation_type(project, my_materialized_view) == "materialized_view" + @pytest.mark.flaky + def test_table_replaces_materialized_view(self, project, my_materialized_view): + super().test_table_replaces_materialized_view(project, my_materialized_view) + + @pytest.mark.flaky + def test_view_replaces_materialized_view(self, project, my_materialized_view): + super().test_view_replaces_materialized_view(project, my_materialized_view) + class RedshiftMaterializedViewChanges(MaterializedViewChanges): @pytest.fixture(scope="class", autouse=True) @@ -200,6 +208,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) @@ -224,7 +233,10 @@ 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) NO_BACKUP_MATERIALIZED_VIEW = """ @@ -259,6 +271,7 @@ def dbt_run_results(self, project): def test_running_mv_with_backup_false_succeeds(self, dbt_run_results): assert dbt_run_results[0].node.config_call_dict["backup"] is False + @pytest.mark.flaky def test_running_mv_with_backup_false_is_idempotent(self, project, dbt_run_results): """ Addresses: https://github.com/dbt-labs/dbt-redshift/issues/621 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