Repository: apache/airflow (language: python)

Issue:
Split tests to core/providers/task-sdk/integration/system
The tests execution was traditionally using single "breeze testing tests" command and you could select which test to run via TEST_TYPE.

However the recent move of providers and adding task_sdk necessitates splitting the tests commands into separate commands for core, providers, task_sdk, helm, integration and system.

This is done via introducing "TEST_GROUP" - which determines which group of tests is being executed, and dedicated testing command for each of the groups - with "db" and "non-db" variants where applicable.

Cleanup and small refactoring has been done to make it easier to reason about parameters passed down from the command line to docker and in-container pytest command.

Related: #42632

<!--
 Licensed to the Apache Software Foundation (ASF) under one
 or more contributor license agreements.  See the NOTICE file
 distributed with this work for additional information
 regarding copyright ownership.  The ASF licenses this file
 to you under the Apache License, Version 2.0 (the
 "License"); you may not use this file except in compliance
 with the License.  You may obtain a copy of the License at

   http://www.apache.org/licenses/LICENSE-2.0

 Unless required by applicable law or agreed to in writing,
 software distributed under the License is distributed on an
 "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
 KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->

<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.

Feel free to ping committers for the review!

In case of an existing issue, reference it using one of the following:

closes: #ISSUE
related: #ISSUE

How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->



<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).



Discussion / hints:
OK. I moved it here from https://github.com/apache/airflow/pull/43965 .

That one should be ready for review. I got it green, all the tests are nicely split between core and providers. I will be adding a little more diagnostics (Right now it is not obvious if the successful tests are doing what they are supposed to do) and running it for all versions of Python etc. to check that everythong is as expected + I will add runnig system tests - but other than that, I think it is in the shape that is pretty much ready to merge (and for sure ready to review).

Leter we can run a number optimizations, but the way it is and after I check it for all typos and run in multiple versions, we should be ready to go. I am quite away next week (in Ireland) - and will be less available again in the next week, but I am confident it's pretty stable (but will be able to fix things when on/off and with @gopidesupavan @romsharon98 @shahar1 @amoghrajesh @tirkarthi  @ashb @kaxil @pierrejeambrun @vincbeck @o-nikolas and others who contributed recently, I think this way of running tests  should be easier to reason about and fix.

Free screenshots to show the current state:

The "core" and "providers" tests now run in completely separate now - following the 'Stage 1` of https://github.com/apache/airflow/issues/42632#issuecomment-2449671014: 

![image](https://github.com/user-attachments/assets/2c08b221-4a81-46a8-ae0b-06d390392d10)

List of tests commands:

![image](https://github.com/user-attachments/assets/523e5e74-3106-42b1-bd88-427e765d9280)

One thing I am not sure here is naming. Should it be:

* `integration-providers-tests`

or

* `providers-integration-tests`

Both have it's rationale, it's mostly about whether we want to group them by "type" or "what sub-system they are working on". Would love to have other's opinion on that.

Any and all comments are welcome! Yes. It's huge, but a lot of it is auto-generated and you can just mark it as "read" and it will go away.



Just so I understand the plan, this splits out the running of tests, but doesn't change to code location which will be a future PR?
Interface question should the db/non-db be a flag that operates like a filter, so something like `breeze testing core-tests --non-db` etc?

Of particular note, I _imagine_ that soon (as part of TaskSDK work) all the db unit tests for the providers will go away/become non-db tests. I don't know if that changes your thinking at all
` +6,413 / −3,614 ` 😱 

Oh, this is mostly in the SVG images isn't it
> Interface question should the db/non-db be a flag that operates like a filter, so something like `breeze testing core-tests --non-db` etc?
> 
> Of particular note, I _imagine_ that soon (as part of TaskSDK work) all the db unit tests for the providers will go away/become non-db tests. I don't know if that changes your thinking at all

I agree, a flag to filter db/non-db tests would make more sense to me instead of having separate commands
> Oh, this is mostly in the SVG images isn't it

Yeah. It's definitely less code than it was :)
@ashb 
> Interface question should the db/non-db be a flag that operates like a filter, so something like breeze testing core-tests --non-db etc?

@vincbeck 
> 
> I agree, a flag to filter db/non-db tests would make more sense to me instead of having separate commands

OK. That's the 3rd similar comment after @o-nikolas - and I explained the reasons in: https://github.com/apache/airflow/pull/43965#issuecomment-2474956968 

So if everyone around you seems to be mad, maybe it's you :) 

It's really additional level of abstraction for the base command that makes it easier to run and has some defaults set differently, but it is all possible to be done with the base command - it's just a matter to pass a few more parameters.  And it was meant as "easier to reproduce locally" - in CI I can easily make those commands more complex.

Let me try to remove it and see how it will be then, that's not a big change and will decrease amount of code.
> Of particular note, I imagine that soon (as part of TaskSDK work) all the db unit tests for the providers will go away/become non-db tests. I don't know if that changes your thinking at all

That's planned  in  stage 3 in https://github.com/apache/airflow/issues/42632#issuecomment-2449671014 - but it's pretty independent - there we would simply remove the db method and rename it, but yeah, I will try to remove `db/non-db` commands now.
> 
Just so I understand the plan, this splits out the running of tests, but doesn't change to code location which will be a future PR?

Absolutely. One thing at  a time. We want to merge it, let it run for 2-3 weeks and fix all teething problems before we go to next stages. Same as with the past "providers" move. We will find things a week or two from now after we merge this one, I am quite sure of that. And we have to do it carefully while not breaking too much the workflows of people who work noow with a speed of litght -  in parallel.


The whole  set of test commands is simplified now:

<img width="971" alt="Screenshot 2024-11-15 at 02 21 55" src="https://github.com/user-attachments/assets/1c865a38-08bf-4258-aed6-66d7a91eefed">

I also updated docs and examples and contributing docs.

I also reviewed and updated docs and fixed and simplified how sytem tests are run.

There is no more `--system SYSTEM` or `pytest.mark.system("SYSTEM")` but simply `--system`  and `pytest.mark.system`. Also the example dags in "tests/system" and "providers/tests/system" are automatically marked with the "system" marker.

So tests shoudl be run:

In venv/inside breeze:

```bash
pytest --system providers/tests/system/google/cloud/bigquery/example_bigquery_queries.py
```

via Breeze - there is one command only:

```bash
breeze testing system-tests  providers/tests/system/google/cloud/bigquery/example_bigquery_queries.py
```

cc: @ahidalgob @kosteev @pankajkoti @fdemiane @sc250072 

nce we merge it you will have to update the dashboards
And merged!
Thanks for heads up, Jarek.
We will update System Tests Dashboard for google provider package with regard to these changes.

Fix patch:
diff --git a/Dockerfile.ci b/Dockerfile.ci
index 05233f06419e4..8fb36054eed4b 100644
--- a/Dockerfile.ci
+++ b/Dockerfile.ci
@@ -1080,7 +1080,7 @@ function check_run_tests() {
     if [[ ${REMOVE_ARM_PACKAGES:="false"} == "true" ]]; then
         # Test what happens if we do not have ARM packages installed.
         # This is useful to see if pytest collection works without ARM packages which is important
-        # for the MacOS M1 users running tests in their ARM machines with `breeze testing tests` command
+        # for the MacOS M1 users running tests in their ARM machines with `breeze testing *-tests` command
         python "${IN_CONTAINER_DIR}/remove_arm_packages.py"
     fi
 
diff --git a/contributing-docs/05_pull_requests.rst b/contributing-docs/05_pull_requests.rst
index 1e14167943497..db25643d74bae 100644
--- a/contributing-docs/05_pull_requests.rst
+++ b/contributing-docs/05_pull_requests.rst
@@ -88,7 +88,8 @@ these guidelines:
     to the changed code (for example for ``airflow/cli/cli_parser.py`` changes you have tests in
     ``tests/cli/test_cli_parser.py``). However there are a number of cases where the tests that should run
     are placed elsewhere - you can either run tests for the whole ``TEST_TYPE`` that is relevant (see
-    ``breeze testing tests --help`` output for available test types) or you can run all tests, or eventually
+    ``breeze testing core-tests --help`` or ``breeze testing providers-tests --help`` output for
+    available test types for each of the testing commands) or you can run all tests, or eventually
     you can push your code to PR and see results of the tests in the CI.
 
 -   You can use any supported python version to run the tests, but the best is to check
diff --git a/dev/breeze/doc/03_developer_tasks.rst b/dev/breeze/doc/03_developer_tasks.rst
index 87bb2713b93fa..ad1a4fe0a6f99 100644
--- a/dev/breeze/doc/03_developer_tasks.rst
+++ b/dev/breeze/doc/03_developer_tasks.rst
@@ -419,7 +419,7 @@ are several reasons why you might want to do that.
 
 Breeze uses docker images heavily and those images are rebuild periodically and might leave dangling, unused
 images in docker cache. This might cause extra disk usage. Also running various docker compose commands
-(for example running tests with ``breeze testing tests``) might create additional docker networks that might
+(for example running tests with ``breeze testing core-tests``) might create additional docker networks that might
 prevent new networks from being created. Those networks are not removed automatically by docker-compose.
 Also Breeze uses it's own cache to keep information about all images.
 
diff --git a/scripts/docker/entrypoint_ci.sh b/scripts/docker/entrypoint_ci.sh
index 5ff608adeab70..956b18a666b51 100755
--- a/scripts/docker/entrypoint_ci.sh
+++ b/scripts/docker/entrypoint_ci.sh
@@ -316,7 +316,7 @@ function check_run_tests() {
     if [[ ${REMOVE_ARM_PACKAGES:="false"} == "true" ]]; then
         # Test what happens if we do not have ARM packages installed.
         # This is useful to see if pytest collection works without ARM packages which is important
-        # for the MacOS M1 users running tests in their ARM machines with `breeze testing tests` command
+        # for the MacOS M1 users running tests in their ARM machines with `breeze testing *-tests` command
         python "${IN_CONTAINER_DIR}/remove_arm_packages.py"
     fi
 


Test patch:
diff --git a/dev/breeze/doc/05_test_commands.rst b/dev/breeze/doc/05_test_commands.rst
index 3ce2f366db9b9..f5b3a78eef78f 100644
--- a/dev/breeze/doc/05_test_commands.rst
+++ b/dev/breeze/doc/05_test_commands.rst
@@ -106,9 +106,9 @@ For example this will run API and WWW tests in parallel:
 
 .. code-block:: bash
 
-    breeze testing tests --parallel-test-types "API WWW" --run-in-parallel
+    breeze testing core-tests --parallel-test-types "API WWW" --run-in-parallel
 
-Here is the detailed set of options for the ``breeze testing tests`` command.
+Here is the detailed set of options for the ``breeze testing core-tests`` command.
 
 .. image:: ./images/output_testing_core-tests.svg
   :target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/images/output_testing_core-tests.svg
@@ -143,11 +143,11 @@ You can also run parallel tests with ``--run-in-parallel`` flag - by default it
 in parallel, but you can specify the test type that you want to run with space separated list of test
 types passed to ``--parallel-test-types`` flag.
 
-For example this will run API and WWW tests in parallel:
+For example this will run ``amazon`` and ``google`` tests in parallel:
 
 .. code-block:: bash
 
-    breeze testing tests --parallel-test-types "Providers[amazon] Providers[google]" --run-in-parallel
+    breeze testing providers-tests --parallel-test-types "Providers[amazon] Providers[google]" --run-in-parallel
 
 Here is the detailed set of options for the ``breeze testing providers-test`` command.