[airflow] 01/01: Print important information outside of the folded output in CI

[po...@apache.org]

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

potiuk pushed a commit to branch better-information-in-ci-output
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit 87945390dcae7041e9eb92a169d7989b60437bce
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Tue Oct 25 13:38:50 2022 +0200

    Print important information outside of the folded output in CI
    
    This PR improves the ways important information ise printed in CI.
    
    Currently some of the errors and important information is
    printed inside the folded console output when they are run. This
    is nice to have the folded content in case of success but in case of
    error we want to surface errors directly in the CI output so that the
    user does not have to find and unfold the information. Sometimes
    (for example in printing which dependencies got upgraded) we also
    want to surface some important part of the output even if there is
    a success.
    
    This is a foundational change that makes it possible and easy, we are
    going to add more of those as we find them.
    
    This change improves surfacing of the information in three cases:
    
    1) Test failures
    
    When there is an error in tests, we should now see the (only the part
    containing error and traceback) see the failure details outside of the
    folded content - the folded content will still contain all the details
    of the test run, but you would not have to unfold it in order to see the
    actual error.
    
    2) Errors when testing provider installation
    
    When there are errors when installing providers, the errors were
    initially inside of the folded "run" command and the run command did not
    even have indication that there was an error (just exit code information
    was printed). This change ends CI group when errors are detected (in CI)
    which makes the errors show up outside of the group.
    
    3) Showing changed constraints
    
    Showing changed constraints when upgrading to newer dependencies is
    always interesting - regardless from being it error or success.
    With this change, we are showing just the constraints printed in the
    main logs.
---
 .../commands/release_management_commands.py         |  2 ++
 .../src/airflow_breeze/commands/testing_commands.py |  1 +
 dev/breeze/src/airflow_breeze/utils/parallel.py     | 21 ++++++++++++++++++++-
 scripts/in_container/verify_providers.py            |  4 ++++
 tests/always/test_connection.py                     |  1 +
 5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/dev/breeze/src/airflow_breeze/commands/release_management_commands.py b/dev/breeze/src/airflow_breeze/commands/release_management_commands.py
index 26cafb8f97..1a2a444e90 100644
--- a/dev/breeze/src/airflow_breeze/commands/release_management_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/release_management_commands.py
@@ -363,6 +363,8 @@ def run_generate_constraints_in_parallel(
         outputs=outputs,
         include_success_outputs=include_success_outputs,
         skip_cleanup=skip_cleanup,
+        print_summary_after_line_matching=".*Constraints generated in.*",
+        print_summary_also_after_success=True,
     )
 
 
diff --git a/dev/breeze/src/airflow_breeze/commands/testing_commands.py b/dev/breeze/src/airflow_breeze/commands/testing_commands.py
index 3d8e962180..b74aa548f6 100644
--- a/dev/breeze/src/airflow_breeze/commands/testing_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/testing_commands.py
@@ -263,6 +263,7 @@ def _run_tests_in_pool(
         outputs=outputs,
         include_success_outputs=include_success_outputs,
         skip_cleanup=skip_cleanup,
+        print_summary_after_line_matching=r".*= FAILURES.*|.*= ERRORS.*",
     )
 
 
diff --git a/dev/breeze/src/airflow_breeze/utils/parallel.py b/dev/breeze/src/airflow_breeze/utils/parallel.py
index 7ee2d6c2a8..4e6175bc80 100644
--- a/dev/breeze/src/airflow_breeze/utils/parallel.py
+++ b/dev/breeze/src/airflow_breeze/utils/parallel.py
@@ -65,7 +65,7 @@ def nice_timedelta(delta: datetime.timedelta):
 ANSI_COLOUR_MATCHER = re.compile(r"(?:\x1B[@-_]|[\x80-\x9F])[0-?]*[ -/]*[@-~]")
 
 
-def remove_ansi_colours(line):
+def remove_ansi_colours(line: str):
     return ANSI_COLOUR_MATCHER.sub("", line)
 
 
@@ -343,6 +343,8 @@ def check_async_run_results(
     include_success_outputs: bool,
     poll_time: float = 0.2,
     skip_cleanup: bool = False,
+    print_summary_after_line_matching: str | None = None,
+    print_summary_also_after_success: bool = False,
 ):
     """
     Check if all async results were success. Exits with error if not.
@@ -352,6 +354,11 @@ def check_async_run_results(
     :param include_success_outputs: include outputs of successful parallel runs
     :param poll_time: what's the poll time between checks
     :param skip_cleanup: whether to skip cleanup of temporary files.
+    :param print_summary_after_line_matching: if it is not None, the regexp determines line after which
+        outputs should be printed as summary, so that you do not have to look at the folded details of
+        the run in CI
+    :param print_summary_also_after_success: if summary is printed, this flag determines if summaries should
+        be only printed on failure (False) or also on success (True).
     """
     from airflow_breeze.utils.ci_group import ci_group
 
@@ -387,6 +394,18 @@ def check_async_run_results(
                 os.write(1, Path(outputs[i].file_name).read_bytes())
         else:
             get_console().print(f"[success]{outputs[i].title}")
+    if print_summary_after_line_matching is not None:
+        regex = re.compile(print_summary_after_line_matching)
+        for i, result in enumerate(results):
+            error = result.get()[0] != 0
+            if error or print_summary_also_after_success:
+                print_lines = False
+                for line in Path(outputs[i].file_name).read_bytes().decode(errors="ignore").splitlines():
+                    if not print_lines and regex.match(remove_ansi_colours(line)):
+                        print_lines = True
+                        get_console().print(f"\n[info]Summary: {outputs[i].title:<30}:\n")
+                    if print_lines:
+                        print(line)
     try:
         if errors:
             get_console().print("\n[error]There were errors when running some tasks. Quitting.[/]\n")
diff --git a/scripts/in_container/verify_providers.py b/scripts/in_container/verify_providers.py
index ad5a6b0f73..a64ed81f47 100755
--- a/scripts/in_container/verify_providers.py
+++ b/scripts/in_container/verify_providers.py
@@ -767,12 +767,16 @@ def summarise_total_vs_bad_and_warnings(total: int, bad: int, warns: list[warnin
         console.print()
     else:
         console.print()
+        if os.environ.get("CI") != "":
+            console.print("::endgroup::")
         console.print(
             f"[red]ERROR! There are in total: {bad} entities badly named out of {total} entities[/]"
         )
         console.print()
         raise_error = True
     if warns:
+        if os.environ.get("CI") != "" and bad == 0:
+            console.print("::endgroup::")
         console.print()
         console.print("[red]Unknown warnings generated:[/]")
         console.print()
diff --git a/tests/always/test_connection.py b/tests/always/test_connection.py
index dac7bb1104..1801299eda 100644
--- a/tests/always/test_connection.py
+++ b/tests/always/test_connection.py
@@ -82,6 +82,7 @@ class TestConnection(unittest.TestCase):
         test_connection = Connection(extra="testextra")
         assert not test_connection.is_extra_encrypted
         assert test_connection.extra == "testextra"
+        assert True is False
 
     @conf_vars({("core", "fernet_key"): Fernet.generate_key().decode()})
     def test_connection_extra_with_encryption(self):