Add clearer exception for read failures in macro plugin upgrade check (…

…#13371)

Resolves #13349

airflow/upgrade/rules/airflow_macro_plugin_removed.py

@@ -39,15 +39,20 @@ def _check_file(self, file_path):
         problems = []
         class_name_to_check = self.MACRO_PLUGIN_CLASS.split(".")[-1]
         with open(file_path, "r") as file_pointer:
-            for line_number, line in enumerate(file_pointer, 1):
-                if class_name_to_check in line:
-                    problems.append(self._change_info(file_path, line_number))
+            try:
+                for line_number, line in enumerate(file_pointer, 1):
+                    if class_name_to_check in line:
+                        problems.append(self._change_info(file_path, line_number))
+            except UnicodeDecodeError:
+                problems.append("Unable to read python file {}".format(file_path))
         return problems
 
     def check(self):
         dag_folder = conf.get("core", "dags_folder")
         file_paths = list_py_file_paths(directory=dag_folder, include_examples=False)
         problems = []
         for file_path in file_paths:
+            if not file_path.endswith(".py"):
+                continue
             problems.extend(self._check_file(file_path))
         return problems

tests/upgrade/rules/test_airflow_macro_plugin_removed.py

@@ -20,12 +20,14 @@
 from tempfile import NamedTemporaryFile
 from tests.compat import mock
 
-from airflow.upgrade.rules.airflow_macro_plugin_removed import AirflowMacroPluginRemovedRule
+from airflow.upgrade.rules.airflow_macro_plugin_removed import (
+    AirflowMacroPluginRemovedRule,
+)
 
 
 @contextmanager
-def create_temp_file(mock_list_files, lines):
-    with NamedTemporaryFile("w+") as temp_file:
+def create_temp_file(mock_list_files, lines, extension=".py"):
+    with NamedTemporaryFile("w+", suffix=extension) as temp_file:
         mock_list_files.return_value = [temp_file.name]
         temp_file.writelines("\n".join(lines))
         temp_file.flush()
@@ -34,13 +36,16 @@ def create_temp_file(mock_list_files, lines):
 
 @mock.patch("airflow.upgrade.rules.airflow_macro_plugin_removed.list_py_file_paths")
 class TestAirflowMacroPluginRemovedRule(TestCase):
+
+    def test_rule_info(self, mock_list_files):
+        rule = AirflowMacroPluginRemovedRule()
+        assert isinstance(rule.description, str)
+        assert isinstance(rule.title, str)
+
     def test_valid_check(self, mock_list_files):
         lines = ["import foo.bar.baz"]
         with create_temp_file(mock_list_files, lines):
             rule = AirflowMacroPluginRemovedRule()
-            assert isinstance(rule.description, str)
-            assert isinstance(rule.title, str)
-
             msgs = rule.check()
             assert 0 == len(msgs)
 
@@ -52,10 +57,6 @@ def test_invalid_check(self, mock_list_files):
         with create_temp_file(mock_list_files, lines) as temp_file:
 
             rule = AirflowMacroPluginRemovedRule()
-
-            assert isinstance(rule.description, str)
-            assert isinstance(rule.title, str)
-
             msgs = rule.check()
             assert 2 == len(msgs)
 
@@ -66,3 +67,27 @@ def test_invalid_check(self, mock_list_files):
                 "{} (line {})".format(base_message, line_number) for line_number in [1, 2]
             ]
             assert expected_messages == msgs
+
+    def test_non_python_file_ignored(self, mock_list_files):
+        lines = [
+            "import airflow.AirflowMacroPlugin",
+            "from airflow import AirflowMacroPlugin",
+        ]
+        with create_temp_file(mock_list_files, lines, extension=".other"):
+            # Although this file "matches", it shouldn't be flagged because
+            # only python files are checked for macros anyway
+            rule = AirflowMacroPluginRemovedRule()
+            msgs = rule.check()
+            assert 0 == len(msgs)
+
+    def test_bad_file_failure(self, mock_list_files):
+        # Write a binary file
+        with NamedTemporaryFile("wb+", suffix=".py") as temp_file:
+            mock_list_files.return_value = [temp_file.name]
+            temp_file.write(b"{\x03\xff\x00d")
+            temp_file.flush()
+
+            rule = AirflowMacroPluginRemovedRule()
+            msgs = rule.check()
+            assert 1 == len(msgs)
+            assert msgs == ["Unable to read python file {}".format(temp_file.name)]