Merge pull request #14731 from ethereum/fix-bytecode-report-ignoring-cli-exit-code

Fix `prepare_report.py` ignoring compiler exit code

Author: cameel

Diff for: scripts/bytecodecompare/prepare_report.py

@@ -183,7 +183,10 @@ def parse_standard_json_output(source_file_name: Path, standard_json_output: str
     return file_report
 
 
-def parse_cli_output(source_file_name: Path, cli_output: str) -> FileReport:
+def parse_cli_output(source_file_name: Path, cli_output: str, exit_code: int) -> FileReport:
+    if exit_code != 0:
+        return FileReport(file_name=source_file_name, contract_reports=None)
+
     # re.split() returns a list containing the text between pattern occurrences but also inserts the
     # content of matched groups in between. It also never omits the empty elements so the number of
     # list items is predictable (3 per match + the text before the first match)
@@ -312,7 +315,7 @@ def run_compiler(
             input=compiler_input,
             encoding='utf8',
             capture_output=True,
-            check=exit_on_error,
+            check=True,
         )
 
         return parse_standard_json_output(Path(source_file_name), process.stdout)
@@ -345,7 +348,7 @@ def run_compiler(
             check=exit_on_error,
         )
 
-        return parse_cli_output(Path(source_file_name), process.stdout)
+        return parse_cli_output(Path(source_file_name), process.stdout, process.returncode)
 
 
 def generate_report(

Diff for: scripts/common/cmdline_helpers.py

@@ -48,7 +48,7 @@ def solc_bin_report(solc_binary: str, input_files: List[Path], via_ir: bool) ->
         (['--via-ir'] if via_ir else []),
         encoding='utf8',
     )
-    return parse_cli_output('', output)
+    return parse_cli_output('', output, 0)
 
 
 def save_bytecode(bytecode_path: Path, reports: FileReport, contract: Optional[str] = None):

Diff for: test/scripts/test_bytecodecompare_prepare_report.py

@@ -487,19 +487,19 @@ def test_parse_cli_output(self):
             ]
         )
 
-        report = parse_cli_output(Path('syntaxTests/scoping/library_inherited2.sol'), LIBRARY_INHERITED2_SOL_CLI_OUTPUT)
+        report = parse_cli_output(Path('syntaxTests/scoping/library_inherited2.sol'), LIBRARY_INHERITED2_SOL_CLI_OUTPUT, 0)
         self.assertEqual(report, expected_report)
 
     def test_parse_cli_output_should_report_error_on_compiler_errors(self):
         expected_report = FileReport(file_name=Path('syntaxTests/pragma/unknown_pragma.sol'), contract_reports=None)
 
-        report = parse_cli_output(Path('syntaxTests/pragma/unknown_pragma.sol'), UNKNOWN_PRAGMA_SOL_CLI_OUTPUT)
+        report = parse_cli_output(Path('syntaxTests/pragma/unknown_pragma.sol'), UNKNOWN_PRAGMA_SOL_CLI_OUTPUT, 0)
         self.assertEqual(report, expected_report)
 
     def test_parse_cli_output_should_report_error_on_empty_output(self):
         expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None)
 
-        self.assertEqual(parse_cli_output(Path('file.sol'), ''), expected_report)
+        self.assertEqual(parse_cli_output(Path('file.sol'), '', 0), expected_report)
 
     def test_parse_cli_output_should_report_missing_bytecode_and_metadata(self):
         compiler_output = dedent("""\
@@ -541,22 +541,25 @@ def test_parse_cli_output_should_report_missing_bytecode_and_metadata(self):
             ]
         )
 
-        self.assertEqual(parse_cli_output(Path('syntaxTests/scoping/library_inherited2.sol'), compiler_output), expected_report)
+        self.assertEqual(
+            parse_cli_output(Path('syntaxTests/scoping/library_inherited2.sol'), compiler_output, 0),
+            expected_report
+        )
 
     def test_parse_cli_output_should_report_error_on_unimplemented_feature_error(self):
         expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None)
 
-        self.assertEqual(parse_cli_output(Path('file.sol'), UNIMPLEMENTED_FEATURE_CLI_OUTPUT), expected_report)
+        self.assertEqual(parse_cli_output(Path('file.sol'), UNIMPLEMENTED_FEATURE_CLI_OUTPUT, 0), expected_report)
 
     def test_parse_cli_output_should_report_error_on_stack_too_deep_error(self):
         expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None)
 
-        self.assertEqual(parse_cli_output(Path('file.sol'), STACK_TOO_DEEP_CLI_OUTPUT), expected_report)
+        self.assertEqual(parse_cli_output(Path('file.sol'), STACK_TOO_DEEP_CLI_OUTPUT, 0), expected_report)
 
     def test_parse_cli_output_should_report_error_on_code_generation_error(self):
         expected_report = FileReport(file_name=Path('file.sol'), contract_reports=None)
 
-        self.assertEqual(parse_cli_output(Path('file.sol'), CODE_GENERATION_ERROR_CLI_OUTPUT), expected_report)
+        self.assertEqual(parse_cli_output(Path('file.sol'), CODE_GENERATION_ERROR_CLI_OUTPUT, 0), expected_report)
 
     def test_parse_cli_output_should_handle_output_from_solc_0_4_0(self):
         expected_report = FileReport(
@@ -571,7 +574,7 @@ def test_parse_cli_output_should_handle_output_from_solc_0_4_0(self):
             ]
         )
 
-        self.assertEqual(parse_cli_output(Path('contract.sol'), SOLC_0_4_0_CLI_OUTPUT), expected_report)
+        self.assertEqual(parse_cli_output(Path('contract.sol'), SOLC_0_4_0_CLI_OUTPUT, 0), expected_report)
 
     def test_parse_cli_output_should_handle_output_from_solc_0_4_8(self):
         expected_report = FileReport(
@@ -588,7 +591,7 @@ def test_parse_cli_output_should_handle_output_from_solc_0_4_8(self):
             ]
         )
 
-        self.assertEqual(parse_cli_output(Path('contract.sol'), SOLC_0_4_8_CLI_OUTPUT), expected_report)
+        self.assertEqual(parse_cli_output(Path('contract.sol'), SOLC_0_4_8_CLI_OUTPUT, 0), expected_report)
 
     def test_parse_cli_output_should_handle_leading_and_trailing_spaces(self):
         compiler_output = (
@@ -606,7 +609,7 @@ def test_parse_cli_output_should_handle_leading_and_trailing_spaces(self):
             ]
         )
 
-        self.assertEqual(parse_cli_output(Path('contract.sol'), compiler_output), expected_report)
+        self.assertEqual(parse_cli_output(Path('contract.sol'), compiler_output, 0), expected_report)
 
     def test_parse_cli_output_should_handle_empty_bytecode_and_metadata_lines(self):
         compiler_output = dedent("""\
@@ -640,7 +643,7 @@ def test_parse_cli_output_should_handle_empty_bytecode_and_metadata_lines(self):
             ]
         )
 
-        self.assertEqual(parse_cli_output(Path('contract.sol'), compiler_output), expected_report)
+        self.assertEqual(parse_cli_output(Path('contract.sol'), compiler_output, 0), expected_report)
 
     def test_parse_cli_output_should_handle_link_references_in_bytecode(self):
         compiler_output = dedent("""\
@@ -662,4 +665,4 @@ def test_parse_cli_output_should_handle_link_references_in_bytecode(self):
         )
         # pragma pylint: enable=line-too-long
 
-        self.assertEqual(parse_cli_output(Path('contract.sol'), compiler_output), expected_report)
+        self.assertEqual(parse_cli_output(Path('contract.sol'), compiler_output, 0), expected_report)