This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 16fddbae83 Improve handling of warnings in CI (#28096)
16fddbae83 is described below
commit 16fddbae83d03c9b3e2d249cc8852fb006c65c3b
Author: Jarek Potiuk <[email protected]>
AuthorDate: Sun Dec 4 22:05:32 2022 +0100
Improve handling of warnings in CI (#28096)
Warnings printed in CI have been making it difficult to see what
is going on (they were taking far too much space after the test
results and GitHub CI UI rendered those multi-line warnings slowly.
Also we did not have the right tools to capture the number and list
of warnings that we should deal with.
We are usign pytest-capture-warnings plugin now that improves
the situation twofold:
* warning summary printed by the plugin in the output is
shorter - each warning is at most one line
* the warning text files are uploaded as artifacts which make them
usable in any kind of approach where we want to attempt to
start an effort to remove all warnings
---
.github/actions/post_tests/action.yml | 6 +++++
.gitignore | 2 +-
.rat-excludes | 3 +++
Dockerfile.ci | 3 +++
TESTING.rst | 3 +++
scripts/docker/entrypoint_ci.sh | 3 +++
scripts/in_container/filter_out_warnings.py | 31 +++++++++++++++++++++++
scripts/in_container/run_ci_tests.sh | 8 ++++--
setup.py | 1 +
tests/providers/slack/hooks/test_slack_webhook.py | 6 ++---
10 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/.github/actions/post_tests/action.yml
b/.github/actions/post_tests/action.yml
index 2e15b01ba8..96bed9211b 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 98c9dc2b23..edd1362f96 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 56c8174837..1e16d61a67 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 149bfc17f4..f2e64830fb 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 d47adc6ad1..b0fc8be512 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
<http://doc.pytest.org/en/latest/assert.html>`_ 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 87bce60116..ca147c61c0 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 0000000000..df27eda7e6
--- /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 efab241fb7..963375d776 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 78d501f495..f5625835ba 100644
--- a/setup.py
+++ b/setup.py
@@ -391,6 +391,7 @@ devel_only = [
# 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 f687675891..0c3527902f 100644
--- a/tests/providers/slack/hooks/test_slack_webhook.py
+++ b/tests/providers/slack/hooks/test_slack_webhook.py
@@ -526,13 +526,13 @@ class TestSlackWebhookHook:
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:"
