From dda4921786031e4b3c12aa6e48f176bbef2e867d Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Thu, 18 Aug 2016 13:47:57 -0700 Subject: [PATCH] exit with correct error code when infer.py fails Summary: The error code was always 1, and was only enabled in crashcontext mode due to a typo. Reviewed By: sblackshear, lazaroclapp Differential Revision: D3735661 fbshipit-source-id: c0bb0f5 --- infer/src/backend/infer.ml | 62 ++++++++++--------- .../build_systems/build_integration_tests.py | 29 ++++++++- .../expected_outputs/fail_report.json | 7 +++ 3 files changed, 67 insertions(+), 31 deletions(-) create mode 100644 infer/tests/build_systems/expected_outputs/fail_report.json diff --git a/infer/src/backend/infer.ml b/infer/src/backend/infer.ml index 8d31a4395..12ec50209 100644 --- a/infer/src/backend/infer.ml +++ b/infer/src/backend/infer.ml @@ -111,33 +111,37 @@ let () = let analysis_is_crashcontext = match Config.analyzer with | Some Crashcontext -> true | _ -> false in - if analysis_is_crashcontext then - (* check whether this is the top-level infer process *) - let top_level_infer = - (* if the '--buck' option was passed, then this is the top level process iff the build command - starts with 'buck' *) - if Config.buck then buck - (* otherwise, we assume javac as the build command and thus only one process *) - else true in - if top_level_infer then - (* if we are the top-level process, then find the output directory and collect all - crashcontext summaries under it in a single crashcontext.json file *) - let root_out_dir = if buck then begin - let project_root = match Config.project_root with - | Some root -> root - | None -> Filename.dirname Config.results_dir in - let buck_out = match Config.buck_out with - | Some dir -> dir - | None -> "buck-out" in - Filename.concat project_root buck_out - end - else Config.results_dir in - match Config.stacktrace with - | None -> failwith "Detected -a crashcontext without --stacktrace, \ - this should have been checked earlier." - | Some s -> Crashcontext.collect_all_summaries root_out_dir s; - if status <> Unix.WEXITED 0 then ( - prerr_endline ("Failed to execute: " ^ (String.concat " " (Array.to_list args_py))) ; - exit 1 + (if analysis_is_crashcontext then + (* check whether this is the top-level infer process *) + let top_level_infer = + (* if the '--buck' option was passed, then this is the top level process iff the build + command starts with 'buck' *) + if Config.buck then buck + (* otherwise, we assume javac as the build command and thus only one process *) + else true in + if top_level_infer then + (* if we are the top-level process, then find the output directory and collect all + crashcontext summaries under it in a single crashcontext.json file *) + let root_out_dir = if buck then begin + let project_root = match Config.project_root with + | Some root -> root + | None -> Filename.dirname Config.results_dir in + let buck_out = match Config.buck_out with + | Some dir -> dir + | None -> "buck-out" in + Filename.concat project_root buck_out + end + else Config.results_dir in + match Config.stacktrace with + | None -> failwith "Detected -a crashcontext without --stacktrace, \ + this should have been checked earlier." + | Some s -> Crashcontext.collect_all_summaries root_out_dir s ); - () + let exit_code = match status with + | Unix.WEXITED i -> i + | _ -> 1 in + if exit_code <> 0 then ( + if not (exit_code = 2 && Config.fail_on_bug) then + prerr_endline ("Failed to execute: " ^ (String.concat " " (Array.to_list args_py))) ; + exit exit_code + ) diff --git a/infer/tests/build_systems/build_integration_tests.py b/infer/tests/build_systems/build_integration_tests.py index db0daf0c0..f6ca49ac8 100755 --- a/infer/tests/build_systems/build_integration_tests.py +++ b/infer/tests/build_systems/build_integration_tests.py @@ -62,6 +62,8 @@ ALL_TESTS = [ 'buck', 'cc1', 'cmake', + 'componentkit', + 'fail', 'gradle', 'javac', 'locale', @@ -114,7 +116,7 @@ def save_report(reports, filename): separators=(',', ': '), sort_keys=True) -def run_analysis(clean_cmds, build_cmds, extra_check, env=None): +def run_analysis(clean_cmds, build_cmds, extra_check, should_fail, env=None): for clean_cmd in clean_cmds: subprocess.check_call(clean_cmd, env=env) @@ -134,7 +136,16 @@ def run_analysis(clean_cmds, build_cmds, extra_check, env=None): mode='w', suffix='.out', prefix='analysis_') as analysis_output: - subprocess.check_call(infer_cmd, stdout=analysis_output, env=env) + try: + subprocess.check_call(infer_cmd, + stdout=analysis_output, env=env) + if should_fail is not None: + # hacky since we should clean up infer-out, etc. as below + # if you made the test fails, this is your punishment + assert False + except subprocess.CalledProcessError, exn: + if exn.returncode != should_fail: + raise json_path = os.path.join(temp_out_dir, REPORT_JSON) found_errors = utils.load_json_from_path(json_path) @@ -228,6 +239,7 @@ def test(name, enabled=None, report_fname=None, extra_check=lambda x: None, + should_fail=None, preprocess=lambda: None, postprocess=lambda errors: errors): """Run a test. @@ -244,6 +256,10 @@ def test(name, - [enabled] whether the test should attempt to run. By default it is enabled if [[name] in [to_test]] - [report_fname] where to find the expected Infer results + - [extra_check] some function that will be given the temporary + results directory as argument + - [should_fail] if not None then running infer is expected to fail + with [should_fail] error code - [preprocess] a function to run before the clean and compile commands. If the function returns something non-None, use that as the compile commands. @@ -251,6 +267,7 @@ def test(name, modify them. It must return an Infer report. Returns [True] if the test ran, [False] otherwise. + """ # python can't into using values of arguments in the default # values of other arguments @@ -282,6 +299,7 @@ def test(name, clean_commands, compile_commands, extra_check=extra_check, + should_fail=should_fail, env=env) original = os.path.join(EXPECTED_OUTPUTS_DIR, report_fname) do_test(postprocess(errors), original) @@ -481,6 +499,13 @@ class BuildIntegrationTest(unittest.TestCase): 'TestIgnoreImports.mm'], 'infer_args': ['--cxx', '--no-filtering']}]) + def test_fail_on_issue(self): + test('fail', '--fail-on-issue flag', + CODETOANALYZE_DIR, + [{'compile': ['clang', '-c', 'hello.c'], + 'infer_args': ['--fail-on-issue']}], + should_fail=2) + def test_pmd_xml_output(self): def pmd_check(infer_out): assert os.path.exists(os.path.join(infer_out, 'report.xml')) diff --git a/infer/tests/build_systems/expected_outputs/fail_report.json b/infer/tests/build_systems/expected_outputs/fail_report.json new file mode 100644 index 000000000..a6ed10eff --- /dev/null +++ b/infer/tests/build_systems/expected_outputs/fail_report.json @@ -0,0 +1,7 @@ +[ + { + "bug_type": "NULL_DEREFERENCE", + "file": "hello.c", + "procedure": "test" + } +] \ No newline at end of file