You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2021/03/23 03:25:59 UTC

[airflow] 12/34: Remove Heisentest category and quarantine test_backfill_depends_on_past (#14756)

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit ee34d63e07ef455215ced86de58570d14cd260bd
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Sat Mar 13 17:57:45 2021 +0100

    Remove Heisentest category and quarantine test_backfill_depends_on_past (#14756)
    
    The whole Backfill class was in Heisentest but only one of those tests
    is problematic nowi: test_backfill_depends_on_past. Therfore it makes
    sense to remove the class from heisentests and move the
    depends_on_past to quarantine.
    
    It turned out that this is the last "Heisentest" and with the
    isolation we have now coming in parallel tests, it turns out that
    Heisentests are not really good way thinking about the tests - running
    them in isolation does not often help, it only makes it more difficult
    to flag the tests as flaky.
    
    The quarantine test_backfill_depends_on_past ihas been captured in
    the #14755 issue - and hopefully we will make an effort to
    de-quarantine some of those tests soon.
    
    (cherry picked from commit 4ce952e7c2550e118b2be14371d9761fa30cdc37)
---
 BREEZE.rst                                   |  4 ++--
 CONTRIBUTORS_QUICK_START.rst                 | 17 -----------------
 PULL_REQUEST_WORKFLOW.rst                    |  3 +--
 TESTING.rst                                  | 13 +------------
 breeze-complete                              |  2 +-
 scripts/ci/selective_ci_checks.sh            |  4 ++--
 scripts/ci/testing/ci_run_airflow_testing.sh |  2 +-
 scripts/in_container/entrypoint_ci.sh        |  7 +------
 tests/conftest.py                            | 19 -------------------
 tests/jobs/test_backfill_job.py              |  2 +-
 10 files changed, 10 insertions(+), 63 deletions(-)

diff --git a/BREEZE.rst b/BREEZE.rst
index 874cb46..d608d5e 100644
--- a/BREEZE.rst
+++ b/BREEZE.rst
@@ -2325,7 +2325,7 @@ This is the current syntax for  `./breeze <./breeze>`_:
   --test-type TEST_TYPE
           Type of the test to run. One of:
 
-                 All,Core,Providers,API,CLI,Integration,Other,WWW,Heisentests,Postgres,MySQL,Helm
+                 All,Core,Providers,API,CLI,Integration,Other,WWW,Postgres,MySQL,Helm
 
           Default: All
 
@@ -2739,7 +2739,7 @@ This is the current syntax for  `./breeze <./breeze>`_:
   --test-type TEST_TYPE
           Type of the test to run. One of:
 
-                 All,Core,Providers,API,CLI,Integration,Other,WWW,Heisentests,Postgres,MySQL,Helm
+                 All,Core,Providers,API,CLI,Integration,Other,WWW,Postgres,MySQL,Helm
 
           Default: All
 
diff --git a/CONTRIBUTORS_QUICK_START.rst b/CONTRIBUTORS_QUICK_START.rst
index 1ced771..9a8398e 100644
--- a/CONTRIBUTORS_QUICK_START.rst
+++ b/CONTRIBUTORS_QUICK_START.rst
@@ -634,13 +634,6 @@ All Tests are inside ./tests directory.
 
   - Types of tests
 
-  .. code-block:: bash
-
-   $ breeze --backend mysql --mysql-version 5.7 --python 3.8 --db-reset --test-type
-      All          CLI          Heisentests  Integration  Other        Providers
-      API          Core         Helm         MySQL        Postgres     WWW
-
-
   - Running specific type of Test
 
   .. code-block:: bash
@@ -650,16 +643,6 @@ All Tests are inside ./tests directory.
 
 - Running Integration test for specific test type
 
-
-  - Types of Integration Tests
-
-  .. code-block:: bash
-
-     $ breeze --backend mysql --mysql-version 5.7 --python 3.8 --db-reset --test-type Core --integration
-
-       all        kerberos   openldap   presto     redis
-       cassandra  mongo      pinot      rabbitmq
-
   - Running an Integration Test
 
   .. code-block:: bash
diff --git a/PULL_REQUEST_WORKFLOW.rst b/PULL_REQUEST_WORKFLOW.rst
index 719e8c5..3e77c53 100644
--- a/PULL_REQUEST_WORKFLOW.rst
+++ b/PULL_REQUEST_WORKFLOW.rst
@@ -112,7 +112,6 @@ pytest markers. They can be found in any of those packages and they can be selec
 pylint custom command line options. See `TESTING.rst <TESTING.rst>`_ for details but those are:
 
 * Integration - tests that require external integration images running in docker-compose
-* Heisentests - tests that are vulnerable to some side effects and are better to be run on their own
 * Quarantined - tests that are flaky and need to be fixed
 * Postgres - tests that require Postgres database. They are only run when backend is Postgres
 * MySQL - tests that require MySQL database. They are only run when backend is MySQL
@@ -165,7 +164,7 @@ The logic implemented for the changes works as follows:
       all changed files. In case there are any files changed, then we assume that some unknown files
       changed (likely from the core of airflow) and in this case we enable all test types above and the
       Core test types - simply because we do not want to risk to miss anything.
-   h) In all cases where tests are enabled we also add Heisentests, Integration and - depending on
+   h) In all cases where tests are enabled we also add Integration and - depending on
       the backend used = Postgres or MySQL types of tests.
 
 10) Quarantined tests are always run when tests are run - we need to run them often to observe how
diff --git a/TESTING.rst b/TESTING.rst
index 1efc63c3..39d4e90 100644
--- a/TESTING.rst
+++ b/TESTING.rst
@@ -177,7 +177,7 @@ kinds of test types:
 
        ./breeze --test-type Providers --db-reset tests
 
-* Special kinds of tests - Integration, Heisentests, Quarantined, Postgres, MySQL, which are marked with pytest
+* Special kinds of tests - Integration, Quarantined, Postgres, MySQL, which are marked with pytest
   marks and for those you need to select the type using test-type switch. If you want to run such tests
   using breeze, you need to pass appropriate ``--test-type`` otherwise the test will be skipped.
   Similarly to the per-directory tests if you do not specify the test or tests to run,
@@ -418,17 +418,6 @@ Those tests are marked with ``@pytest.mark.quarantined`` annotation.
 Those tests are skipped by default. You can enable them with ``--include-quarantined`` flag. You
 can also decide to only run tests with ``-m quarantined`` flag to run only those tests.
 
-Heisen tests
-------------
-
-Some of our tests are Heisentests. This means that they run fine in isolation but when they run together with
-others they might fail the tests (this is likely due to resource consumptions). Therefore we run those tests
-in isolation.
-
-Those tests are marked with ``@pytest.mark.heisentests`` annotation.
-Those tests are skipped by default. You can enable them with ``--include-heisentests`` flag. You
-can also decide to only run tests with ``-m heisentests`` flag to run only those tests.
-
 Running Tests with provider packages
 ====================================
 
diff --git a/breeze-complete b/breeze-complete
index 962aaa6..7789552 100644
--- a/breeze-complete
+++ b/breeze-complete
@@ -36,7 +36,7 @@ _breeze_allowed_kind_versions="v0.8.0"
 _breeze_allowed_mysql_versions="5.7 8"
 _breeze_allowed_postgres_versions="9.6 10 11 12 13"
 _breeze_allowed_kind_operations="start stop restart status deploy test shell k9s"
-_breeze_allowed_test_types="All Core Providers API CLI Integration Other WWW Heisentests Postgres MySQL Helm"
+_breeze_allowed_test_types="All Core Providers API CLI Integration Other WWW Postgres MySQL Helm"
 _breeze_allowed_package_formats="both sdist wheel"
 _breeze_allowed_installation_methods=". apache-airflow"
 
diff --git a/scripts/ci/selective_ci_checks.sh b/scripts/ci/selective_ci_checks.sh
index 4ef07f7..68adf9e 100755
--- a/scripts/ci/selective_ci_checks.sh
+++ b/scripts/ci/selective_ci_checks.sh
@@ -199,7 +199,7 @@ function set_upgrade_to_newer_dependencies() {
 }
 
 
-ALL_TESTS="Always Core Other API CLI Providers WWW Integration Heisentests"
+ALL_TESTS="Always Core Other API CLI Providers WWW Integration"
 readonly ALL_TESTS
 
 function set_outputs_run_everything_and_exit() {
@@ -600,7 +600,7 @@ function calculate_test_types_to_run() {
             echo
             SELECTED_TESTS="${SELECTED_TESTS} WWW"
         fi
-        initialization::ga_output test-types "Always Integration Heisentests ${SELECTED_TESTS}"
+        initialization::ga_output test-types "Always Integration ${SELECTED_TESTS}"
     fi
     start_end::group_end
 }
diff --git a/scripts/ci/testing/ci_run_airflow_testing.sh b/scripts/ci/testing/ci_run_airflow_testing.sh
index b585707..e1f4a04 100755
--- a/scripts/ci/testing/ci_run_airflow_testing.sh
+++ b/scripts/ci/testing/ci_run_airflow_testing.sh
@@ -115,7 +115,7 @@ function prepare_tests_to_run() {
     fi
 
     if [[ -z "${TEST_TYPES=}" ]]; then
-        TEST_TYPES="Core Providers API CLI Integration Other WWW Heisentests"
+        TEST_TYPES="Core Providers API CLI Integration Other WWW"
         echo
         echo "Test types not specified. Running all: ${TEST_TYPES}"
         echo
diff --git a/scripts/in_container/entrypoint_ci.sh b/scripts/in_container/entrypoint_ci.sh
index b99cdc1..da1d383 100755
--- a/scripts/in_container/entrypoint_ci.sh
+++ b/scripts/in_container/entrypoint_ci.sh
@@ -321,7 +321,7 @@ else
     elif [[ ${TEST_TYPE:=""} == "All" || ${TEST_TYPE} == "Quarantined" || \
             ${TEST_TYPE} == "Always" || \
             ${TEST_TYPE} == "Postgres" || ${TEST_TYPE} == "MySQL" || \
-            ${TEST_TYPE} == "Heisentests" || ${TEST_TYPE} == "Long" || \
+            ${TEST_TYPE} == "Long" || \
             ${TEST_TYPE} == "Integration" ]]; then
         SELECTED_TESTS=("${ALL_TESTS[@]}")
     else
@@ -346,11 +346,6 @@ elif [[ ${TEST_TYPE:=""} == "Long" ]]; then
         "-m" "long_running"
         "--include-long-running"
     )
-elif [[ ${TEST_TYPE:=""} == "Heisentests" ]]; then
-    EXTRA_PYTEST_ARGS+=(
-        "-m" "heisentests"
-        "--include-heisentests"
-    )
 elif [[ ${TEST_TYPE:=""} == "Postgres" ]]; then
     EXTRA_PYTEST_ARGS+=(
         "--backend"
diff --git a/tests/conftest.py b/tests/conftest.py
index 98a7d1f..ca8c44b 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -152,11 +152,6 @@ def pytest_addoption(parser):
         action="store_true",
         help="Includes quarantined tests (marked with quarantined marker). They are skipped by default.",
     )
-    group.addoption(
-        "--include-heisentests",
-        action="store_true",
-        help="Includes heisentests (marked with heisentests marker). They are skipped by default.",
-    )
     allowed_trace_sql_columns_list = ",".join(ALLOWED_TRACE_SQL_COLUMNS)
     group.addoption(
         "--trace-sql",
@@ -243,9 +238,6 @@ def pytest_configure(config):
         "markers", "quarantined: mark test that are in quarantine (i.e. flaky, need to be isolated and fixed)"
     )
     config.addinivalue_line(
-        "markers", "heisentests: mark test that should be run in isolation due to resource consumption"
-    )
-    config.addinivalue_line(
         "markers", "credential_file(name): mark tests that require credential file in CREDENTIALS_DIR"
     )
     config.addinivalue_line("markers", "airflow_2: mark tests that works only on Airflow 2.0 / master")
@@ -314,14 +306,6 @@ def skip_quarantined_test(item):
         )
 
 
-def skip_heisen_test(item):
-    for _ in item.iter_markers(name="heisentests"):
-        pytest.skip(
-            "The test is skipped because it has heisentests marker. "
-            "And --include-heisentests flag is passed to pytest. {item}".format(item=item)
-        )
-
-
 def skip_if_integration_disabled(marker, item):
     integration_name = marker.args[0]
     environment_variable_name = "INTEGRATION_" + integration_name.upper()
@@ -378,7 +362,6 @@ def pytest_runtest_setup(item):
 
     include_long_running = item.config.getoption("--include-long-running")
     include_quarantined = item.config.getoption("--include-quarantined")
-    include_heisentests = item.config.getoption("--include-heisentests")
 
     for marker in item.iter_markers(name="integration"):
         skip_if_integration_disabled(marker, item)
@@ -397,8 +380,6 @@ def pytest_runtest_setup(item):
         skip_long_running_test(item)
     if not include_quarantined:
         skip_quarantined_test(item)
-    if not include_heisentests:
-        skip_heisen_test(item)
     skip_if_credential_file_missing(item)
     skip_if_airflow_2_test(item)
 
diff --git a/tests/jobs/test_backfill_job.py b/tests/jobs/test_backfill_job.py
index 9826f18..3139f36 100644
--- a/tests/jobs/test_backfill_job.py
+++ b/tests/jobs/test_backfill_job.py
@@ -56,7 +56,6 @@ logger = logging.getLogger(__name__)
 DEFAULT_DATE = timezone.datetime(2016, 1, 1)
 
 
-@pytest.mark.heisentests
 class TestBackfillJob(unittest.TestCase):
     def _get_dummy_dag(self, dag_id, pool=Pool.DEFAULT_POOL_NAME, task_concurrency=None):
         dag = DAG(dag_id=dag_id, start_date=DEFAULT_DATE, schedule_interval='@daily')
@@ -809,6 +808,7 @@ class TestBackfillJob(unittest.TestCase):
         ti.refresh_from_db()
         assert ti.state == State.SUCCESS
 
+    @pytest.mark.quarantined
     def test_backfill_depends_on_past(self):
         """
         Test that backfill respects ignore_depends_on_past