Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise an error on Airflow Deprecation warnings in tests #38504

Merged
merged 12 commits into from
Mar 30, 2024

Conversation

Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Mar 26, 2024

Experiment to show how many tests are not handled deprecation warnings

After spending some time I've collect all tests which raise one of the selected warning during test runs

  • airflow.exceptions.AirflowProviderDeprecationWarning
  • airflow.exceptions.RemovedInAirflow3Warning
  • airflow.utils.context.AirflowContextDeprecationWarning

With some hacks and tricks I've enable validation that this warning not raised (e.g. not captured by pytest.warn)
Unfortunetly this action require to slightly change current warning collection and use only warnings.catch_warnings and default warnings.filterwarnings / warnings.simplefilter

Some side affect might happen when change one part it affect other parts which do not run into the tests, as result temporary broken main, solution for this one pretty simple: make changes as follow up commit / revert commit / add it into the exclusion list tests/deprecations_ignore.yml (last resort)

Simple snippet for collect list of test from pytest error summary
import re

node_pattern = re.compile(r"::")

raw = """
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_get_conn_host - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_get_conn_host_alternative_port - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_get_conn_mode - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_get_conn_events - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_get_conn_purity - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_set_thick_mode_extra - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_set_thick_mode_extra_str - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_set_thick_mode_params - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_thick_mode_defaults_to_false - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_thick_mode_dirs_defaults - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_oracledb_defaults_attributes_default_values - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_set_oracledb_defaults_attributes_extra - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_set_oracledb_defaults_attributes_extra_str - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.
FAILED tests/providers/oracle/hooks/test_oracle.py::TestOracleHookConn::test_set_oracledb_defaults_attributes_params - airflow.exceptions.AirflowProviderDeprecationWarning: Using conn.schema to pass the Oracle Service Name is deprecated.
                        Please use conn.extra.service_name instead.



""".strip().splitlines()

failed_tests = set()
for line in raw:
    line = line.strip()
    if not line.startswith(("FAILED", "ERROR")):
        continue
    node_id = line.split(" ", 3)[1]
    node_parts = node_pattern.split(node_id)
    last_id, *_ = node_parts[-1].partition("[")
    node_parts[-1] = last_id
    failed_tests.add("::".join(node_parts))

for test in sorted(failed_tests):
    print(f'- {test}')

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

@Taragolis Taragolis added the full tests needed We need to run full set of tests for this PR to merge label Mar 26, 2024
@Taragolis Taragolis force-pushed the poc-deprecation-warnings-errors branch from 9d44501 to 922d1ac Compare March 26, 2024 15:52
@Taragolis Taragolis closed this Mar 26, 2024
@Taragolis Taragolis reopened this Mar 26, 2024
tests/conftest.py Outdated Show resolved Hide resolved
@Taragolis Taragolis force-pushed the poc-deprecation-warnings-errors branch from 0a23e70 to 0d0ce60 Compare March 27, 2024 21:54
@Taragolis Taragolis added the all versions If set, the CI build will be forced to use all versions of Python/K8S/DBs label Mar 28, 2024
@Taragolis Taragolis force-pushed the poc-deprecation-warnings-errors branch from 5f4f14b to 1c3e38b Compare March 28, 2024 11:38
@Taragolis
Copy link
Contributor Author

Not related to this PR, just for follow up found where in tests we use masking in logs "test". It has a flacky behaviour, and depend on which xdist worker tests comes

FAILED tests/providers/google/cloud/hooks/test_dataflow.py::TestDataflow::test_dataflow_wait_for_done_logging - AssertionError: assert 'Running command: test cmd' in ['Running command: *** cmd', 'Start waiting for Apache Beam process to complete.', 'Waiting for Apache Beam process to complete.', '***-stdout', 'Process exited with return code: 1']
= 1 failed, 10732 passed, 8670 skipped, 2 xfailed, 5331 warnings in 403.14s (0:06:43) =

@potiuk
Copy link
Member

potiuk commented Mar 28, 2024

Not related to this PR, just for follow up found where in tests we use masking in logs "test". It has a flacky behaviour, and depend on which xdist worker tests comes

Easiest fix -> turn it in to a DB test.

@Taragolis Taragolis force-pushed the poc-deprecation-warnings-errors branch from 2903608 to 2e6ed9d Compare March 28, 2024 19:00
@Taragolis Taragolis changed the title POC: Raise an error on Airflow Deprecation warnings in tests Raise an error on Airflow Deprecation warnings in tests Mar 28, 2024
@Taragolis Taragolis marked this pull request as ready for review March 28, 2024 19:12
@Taragolis Taragolis force-pushed the poc-deprecation-warnings-errors branch from 2e6ed9d to c8437a8 Compare March 28, 2024 19:59
@Taragolis
Copy link
Contributor Author

One thing what confuse me, that why special tests skipped 🤔

@potiuk
Copy link
Member

potiuk commented Mar 29, 2024

One thing what confuse me, that why special tests skipped 🤔

We are currently skipping them for regular runs - in one of the recent optimization I changed them to be run only:

a) in canary builds
b) when upgrade-to-newer-depenendencies are detected (or set via label).

I figured that we will note it relatively quickly when once of those special tests are broken (like I did with Pydantic test few moments ago) and that usually it will require maintainer stepping in anyway. We have quite a number of those special tests and it adds more than 2x number of tests for regular PRs (most of them are running only for Python 3.8 so just 4 set of tests) - so it's a really nice trade-off to only run special tests for those "special" PRs and "catch-all" in canary runs.

@potiuk
Copy link
Member

potiuk commented Mar 29, 2024

so way to trigger the special tests is to add "upgrade to newer dependencies" . Let me do it now.

@potiuk potiuk added the upgrade to newer dependencies If set, upgrade to newer dependencies is forced label Mar 29, 2024
@potiuk potiuk closed this Mar 29, 2024
@potiuk potiuk reopened this Mar 29, 2024
@potiuk
Copy link
Member

potiuk commented Mar 29, 2024

BTW. This one #38604 - will turn such "skipped" special tests into a single line in output, rather than showing all the skipped sub-workflows.

@potiuk
Copy link
Member

potiuk commented Mar 29, 2024

Just realized. Maybe we should add a special if for just "full tests needed" - that would be also a better criteria/way to trigger the special tests.

@potiuk
Copy link
Member

potiuk commented Mar 29, 2024

Added it to the #38604 - once we merge it, full tests needed should also trigger special tests.

@Taragolis Taragolis force-pushed the poc-deprecation-warnings-errors branch from c8437a8 to 2b696f2 Compare March 29, 2024 20:06
@potiuk potiuk merged commit 50a4c95 into apache:main Mar 30, 2024
92 checks passed
@Taragolis Taragolis deleted the poc-deprecation-warnings-errors branch March 30, 2024 04:36
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Mar 30, 2024
ephraimbuddy pushed a commit that referenced this pull request Apr 2, 2024
* POC: Raise an error on Airflow Deprecation warnings in tests

* Add fixture for check deprecations

* Ignore providers tests per test

* Rever changes in test_pod.py

* Add categories for tests, and evaluate most of the tests

* Exclude TestSetDagRunNote::test_should_respond_200_with_anonymous_user

* Check other groups

* Ignore FAB / Connexion 2 specific errors

* Remove non-compliant DbApiHook deprecations

* Disable buitin warning summary and buitify output

* Add information about prohibited warnings

* Add to ignore additional tests from tests/models/test_taskinstance.py::TestTaskInstance

(cherry picked from commit 50a4c95)
ephraimbuddy pushed a commit that referenced this pull request Apr 2, 2024
* POC: Raise an error on Airflow Deprecation warnings in tests

* Add fixture for check deprecations

* Ignore providers tests per test

* Rever changes in test_pod.py

* Add categories for tests, and evaluate most of the tests

* Exclude TestSetDagRunNote::test_should_respond_200_with_anonymous_user

* Check other groups

* Ignore FAB / Connexion 2 specific errors

* Remove non-compliant DbApiHook deprecations

* Disable buitin warning summary and buitify output

* Add information about prohibited warnings

* Add to ignore additional tests from tests/models/test_taskinstance.py::TestTaskInstance

(cherry picked from commit 50a4c95)
mathiaHT pushed a commit to mathiaHT/airflow that referenced this pull request Apr 4, 2024
* POC: Raise an error on Airflow Deprecation warnings in tests

* Add fixture for check deprecations

* Ignore providers tests per test

* Rever changes in test_pod.py

* Add categories for tests, and evaluate most of the tests

* Exclude TestSetDagRunNote::test_should_respond_200_with_anonymous_user

* Check other groups

* Ignore FAB / Connexion 2 specific errors

* Remove non-compliant DbApiHook deprecations

* Disable buitin warning summary and buitify output

* Add information about prohibited warnings

* Add to ignore additional tests from tests/models/test_taskinstance.py::TestTaskInstance
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
* POC: Raise an error on Airflow Deprecation warnings in tests

* Add fixture for check deprecations

* Ignore providers tests per test

* Rever changes in test_pod.py

* Add categories for tests, and evaluate most of the tests

* Exclude TestSetDagRunNote::test_should_respond_200_with_anonymous_user

* Check other groups

* Ignore FAB / Connexion 2 specific errors

* Remove non-compliant DbApiHook deprecations

* Disable buitin warning summary and buitify output

* Add information about prohibited warnings

* Add to ignore additional tests from tests/models/test_taskinstance.py::TestTaskInstance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all versions If set, the CI build will be forced to use all versions of Python/K8S/DBs changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge upgrade to newer dependencies If set, upgrade to newer dependencies is forced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants