[airflow] branch master updated: Fix selective checks for changes outside of airflow .py files (#13430)

[po...@apache.org]

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

potiuk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/master by this push:
     new 1fe83a4  Fix selective checks for changes outside of airflow .py files (#13430)
1fe83a4 is described below

commit 1fe83a435df0114df3e274e5e79b52ebaef8ac64
Author: Jarek Potiuk <ja...@polidea.com>
AuthorDate: Sat Jan 2 12:33:03 2021 +0100

    Fix selective checks for changes outside of airflow .py files (#13430)
    
    When no airflow files change, selective tests only run basic
    tests, but this is wrong, because many of .py files are
    outside of the airflow folder.
    
    In this case we should enable image building because only then
    full set of static checks is executed.
    
    This bug caused for example #13403 to succeed even if it failed
    static checks after merge.
---
 PULL_REQUEST_WORKFLOW.rst         | 20 +++++++++++---------
 scripts/ci/selective_ci_checks.sh | 21 ++++++++++++++++++++-
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/PULL_REQUEST_WORKFLOW.rst b/PULL_REQUEST_WORKFLOW.rst
index ea10488..39ef618 100644
--- a/PULL_REQUEST_WORKFLOW.rst
+++ b/PULL_REQUEST_WORKFLOW.rst
@@ -136,17 +136,19 @@ The logic implemented for the changes works as follows:
    files), then we again run all tests and checks. Those are cases where the logic of the checks changed
    or the environment for the checks changed so we want to make sure to check everything.
 
-4) If any of docs changed: we need to have CI image so we enable image building
+4) If any of py files changed: we need to have CI image and run full static checks so we enable image building
 
-5) If any of chart files changed, we need to run helm tests so we enable helm unit tests
+5) If any of docs changed: we need to have CI image so we enable image building
 
-6) If any of API files changed, we need to run API tests so we enable them
+6) If any of chart files changed, we need to run helm tests so we enable helm unit tests
 
-7) If any of the relevant source files that trigger the tests have changed at all. Those are airflow
+7) If any of API files changed, we need to run API tests so we enable them
+
+8) If any of the relevant source files that trigger the tests have changed at all. Those are airflow
    sources, chart, tests and kubernetes_tests. If any of those files changed, we enable tests and we
    enable image building, because the CI images are needed to run tests.
 
-8) Then we determine which types of the tests should be run. We count all the changed files in the
+9) Then we determine which types of the tests should be run. We count all the changed files in the
    relevant airflow sources (airflow, chart, tests, kubernetes_tests) first and then we count how many
    files changed in different packages:
 
@@ -166,11 +168,11 @@ The logic implemented for the changes works as follows:
    h) In all cases where tests are enabled we also add Heisentests, Integration and - depending on
       the backend used = Postgres or MySQL types of tests.
 
-9) Quarantined tests are always run when tests are run - we need to run them often to observe how
-   often they fail so that we can decide to move them out of quarantine. Details about the
-   Quarantined tests are described in `TESTING.rst <TESTING.rst>`_
+10) Quarantined tests are always run when tests are run - we need to run them often to observe how
+    often they fail so that we can decide to move them out of quarantine. Details about the
+    Quarantined tests are described in `TESTING.rst <TESTING.rst>`_
 
-10) There is a special case of static checks. In case the above logic determines that the CI image
+11) There is a special case of static checks. In case the above logic determines that the CI image
     needs to be build, we run long and more comprehensive version of static checks - including Pylint,
     MyPy, Flake8. And those tests are run on all files, no matter how many files changed.
     In case the image is not built, we run only simpler set of changes - the longer static checks
diff --git a/scripts/ci/selective_ci_checks.sh b/scripts/ci/selective_ci_checks.sh
index d00a2fe..314f59f 100755
--- a/scripts/ci/selective_ci_checks.sh
+++ b/scripts/ci/selective_ci_checks.sh
@@ -256,7 +256,7 @@ function set_output_skip_tests_but_build_images_and_exit() {
     run_tests "false"
     run_kubernetes_tests "false"
     set_test_types ""
-    set_basic_checks_only "true"
+    set_basic_checks_only "false"
     set_docs_build "true"
     set_image_build "true"
     exit
@@ -394,6 +394,24 @@ function check_if_docs_should_be_generated() {
     start_end::group_end
 }
 
+
+ANY_PY_FILES_CHANGED=(
+    "\.py$"
+)
+readonly ANY_PY_FILES_CHANGED
+
+function check_if_any_py_files_changed() {
+    start_end::group_start "Check if any Python files changed"
+    local pattern_array=("${ANY_PY_FILES_CHANGED[@]}")
+    show_changed_files
+
+    if [[ $(count_changed_files) != "0" ]]; then
+        image_build_needed="true"
+    fi
+    start_end::group_end
+}
+
+
 AIRFLOW_SOURCES_TRIGGERING_TESTS=(
     "^airflow"
     "^chart"
@@ -597,6 +615,7 @@ kubernetes_tests_needed="false"
 
 get_changed_files
 run_all_tests_if_environment_files_changed
+check_if_any_py_files_changed
 check_if_docs_should_be_generated
 check_if_helm_tests_should_be_run
 check_if_api_tests_should_be_run