diff --git a/.github/actions/post_tests/action.yml b/.github/actions/post_tests/action.yml index 2e15b01ba8bdb..96bed9211bb5b 100644 --- a/.github/actions/post_tests/action.yml +++ b/.github/actions/post_tests/action.yml @@ -42,6 +42,12 @@ runs: name: coverage-${{env.JOB_ID}} path: ./files/coverage*.xml retention-days: 7 + - name: "Upload artifact for warnings" + uses: actions/upload-artifact@v3 + with: + name: test-warnings-${{env.JOB_ID}} + path: ./files/warnings-*.txt + retention-days: 7 - name: "Fix ownership" shell: bash run: breeze ci fix-ownership diff --git a/.gitignore b/.gitignore index 98c9dc2b239b2..edd1362f96c3e 100644 --- a/.gitignore +++ b/.gitignore @@ -14,10 +14,10 @@ unittests.db airflow/git_version airflow/www/static/coverage/ airflow/www/*.log - /logs/ airflow-webserver.pid standalone_admin_password.txt +warnings.txt # Byte-compiled / optimized / DLL files __pycache__/ diff --git a/.rat-excludes b/.rat-excludes index 56c81748371c5..1e16d61a67f5d 100644 --- a/.rat-excludes +++ b/.rat-excludes @@ -121,6 +121,9 @@ chart/values_schema.schema.json # Newsfragments are snippets that will be, eventually, consumed into RELEASE_NOTES newsfragments/* +# Warning file generated +warnings.txt + # Dev stuff tests/* scripts/* diff --git a/Dockerfile.ci b/Dockerfile.ci index 149bfc17f44d6..f2e64830fbbd6 100644 --- a/Dockerfile.ci +++ b/Dockerfile.ci @@ -802,6 +802,7 @@ fi set -u export RESULT_LOG_FILE="/files/test_result-${TEST_TYPE/\[*\]/}-${BACKEND}.xml" +export WARNINGS_FILE="/files/warnings-${TEST_TYPE/\[*\]/}-${BACKEND}.txt" EXTRA_PYTEST_ARGS=( "--verbosity=0" @@ -816,6 +817,8 @@ EXTRA_PYTEST_ARGS=( "--setup-timeout=${TEST_TIMEOUT}" "--execution-timeout=${TEST_TIMEOUT}" "--teardown-timeout=${TEST_TIMEOUT}" + "--output=${WARNINGS_FILE}" + "--disable-warnings" # Only display summary for non-expected case # f - failed # E - error diff --git a/TESTING.rst b/TESTING.rst index d47adc6ad11da..b0fc8be512e7c 100644 --- a/TESTING.rst +++ b/TESTING.rst @@ -51,6 +51,9 @@ Follow the guidelines when writing unit tests: * For new tests, use standard "asserts" of Python and ``pytest`` decorators/context managers for testing rather than ``unittest`` ones. See `pytest docs `_ for details. * Use a parameterized framework for tests that have variations in parameters. +* Use with ``pytest.warn`` to capture warnings rather than ``recwarn`` fixture. We are aiming for 0-warning in our + tests, so we run Pytest with ``--disable-warnings`` but instead we have ``pytest-capture-warnings`` plugin that + overrides ``recwarn`` fixture behaviour. **NOTE:** We plan to convert all unit tests to standard "asserts" semi-automatically, but this will be done later in Airflow 2.0 development phase. That will include setUp/tearDown/context managers and decorators. diff --git a/scripts/docker/entrypoint_ci.sh b/scripts/docker/entrypoint_ci.sh index 87bce601160b1..ca147c61c031d 100755 --- a/scripts/docker/entrypoint_ci.sh +++ b/scripts/docker/entrypoint_ci.sh @@ -245,6 +245,7 @@ fi set -u export RESULT_LOG_FILE="/files/test_result-${TEST_TYPE/\[*\]/}-${BACKEND}.xml" +export WARNINGS_FILE="/files/warnings-${TEST_TYPE/\[*\]/}-${BACKEND}.txt" EXTRA_PYTEST_ARGS=( "--verbosity=0" @@ -259,6 +260,8 @@ EXTRA_PYTEST_ARGS=( "--setup-timeout=${TEST_TIMEOUT}" "--execution-timeout=${TEST_TIMEOUT}" "--teardown-timeout=${TEST_TIMEOUT}" + "--output=${WARNINGS_FILE}" + "--disable-warnings" # Only display summary for non-expected case # f - failed # E - error diff --git a/scripts/in_container/filter_out_warnings.py b/scripts/in_container/filter_out_warnings.py new file mode 100644 index 0000000000000..df27eda7e6876 --- /dev/null +++ b/scripts/in_container/filter_out_warnings.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python +# 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. + +from __future__ import annotations + +import fileinput + +suppress = False + +for line in fileinput.input(): + if line.startswith("warnings summary:"): + suppress = True + if line.startswith("All Warning errors can be found in"): + suppress = False + if not suppress: + print(line, end="") diff --git a/scripts/in_container/run_ci_tests.sh b/scripts/in_container/run_ci_tests.sh index efab241fb7711..963375d77645d 100755 --- a/scripts/in_container/run_ci_tests.sh +++ b/scripts/in_container/run_ci_tests.sh @@ -23,10 +23,14 @@ echo "Starting the tests with those pytest arguments:" "${@}" echo set +e -pytest "${@}" - +pytest "${@}" | python "$( dirname "${BASH_SOURCE[0]}" )/filter_out_warnings.py" RES=$? +if [[ -f ${WARNINGS_FILE} ]]; then + echo "Number of warnings: $(wc -l "${WARNINGS_FILE}")" +fi + + if [[ ${RES} == "139" ]]; then echo "${COLOR_YELLOW}Sometimes Pytest fails at exiting with segfault, but all tests actually passed${COLOR_RESET}" echo "${COLOR_YELLOW}We should ignore such case. Checking if junitxml file ${RESULT_LOG_FILE} is there with 0 errors and failures${COLOR_RESET}" diff --git a/setup.py b/setup.py index 78d501f49510b..f5625835ba88b 100644 --- a/setup.py +++ b/setup.py @@ -391,6 +391,7 @@ def write_version(filename: str = str(AIRFLOW_SOURCES_ROOT / "airflow" / "git_ve # TODO: upgrade it and remove the limit "pytest~=6.0", "pytest-asyncio", + "pytest-capture-warnings", "pytest-cov", "pytest-instafail", # We should attempt to remove the limit when we upgrade Pytest diff --git a/tests/providers/slack/hooks/test_slack_webhook.py b/tests/providers/slack/hooks/test_slack_webhook.py index f687675891e7d..0c3527902f86f 100644 --- a/tests/providers/slack/hooks/test_slack_webhook.py +++ b/tests/providers/slack/hooks/test_slack_webhook.py @@ -526,13 +526,13 @@ def test_hook_send_by_hook_attributes(self, mock_hook_send_dict, deprecated_hook mock_hook_send_dict.assert_called_once_with(body=expected_body, headers=None) @mock.patch("airflow.providers.slack.hooks.slack_webhook.WebhookClient") - def test_hook_ignored_attributes(self, mock_webhook_client_cls, recwarn): + def test_hook_ignored_attributes(self, mock_webhook_client_cls): """Test hook constructor warn users about ignored attributes.""" mock_webhook_client = mock_webhook_client_cls.return_value mock_webhook_client_send_dict = mock_webhook_client.send_dict mock_webhook_client_send_dict.return_value = MOCK_WEBHOOK_RESPONSE - - hook = SlackWebhookHook(slack_webhook_conn_id=TEST_CONN_ID, link_names="test-value") + with pytest.warns(UserWarning) as recwarn: + hook = SlackWebhookHook(slack_webhook_conn_id=TEST_CONN_ID, link_names="test-value") assert len(recwarn) == 2 assert str(recwarn.pop(UserWarning).message).startswith( "`link_names` has no affect, if you want to mention user see:"