From 2dbe902ad4364bd908268c0c0270eaeaa902a83d Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Mon, 7 Jan 2019 13:11:25 -0800 Subject: [PATCH] :bug: Fix format_baseline_for_output TypeError Where we accidentally passed a str where we were expecting a dict :snake: Made sure to use kwargs when calling `write_baseline_to_file` Added some docstrings Added `ipdb` to `requirements-dev.txt` --- detect_secrets/core/audit.py | 4 +-- detect_secrets/core/baseline.py | 2 ++ detect_secrets/core/common.py | 7 ++++- detect_secrets/main.py | 21 +++++++++++---- detect_secrets/pre_commit_hook.py | 4 +-- requirements-dev.txt | 1 + tests/core/audit_test.py | 44 +++++++++++++++++++++++-------- tests/main_test.py | 7 +++-- tests/pre_commit_hook_test.py | 4 +-- 9 files changed, 69 insertions(+), 25 deletions(-) diff --git a/detect_secrets/core/audit.py b/detect_secrets/core/audit.py index 4a9878c58..6e62ebfa5 100644 --- a/detect_secrets/core/audit.py +++ b/detect_secrets/core/audit.py @@ -87,8 +87,8 @@ def audit_baseline(baseline_filename): dict(results), ) write_baseline_to_file( - baseline_filename, - original_baseline, + filename=baseline_filename, + data=original_baseline, ) diff --git a/detect_secrets/core/baseline.py b/detect_secrets/core/baseline.py index b2fd57c22..017fce86d 100644 --- a/detect_secrets/core/baseline.py +++ b/detect_secrets/core/baseline.py @@ -172,6 +172,8 @@ def merge_baseline(old_baseline, new_baseline): :type new_baseline: dict :param new_baseline: most recent scan + + :rtype: dict """ new_baseline['results'] = merge_results( old_baseline['results'], diff --git a/detect_secrets/core/common.py b/detect_secrets/core/common.py index 590618ee8..b036272f8 100644 --- a/detect_secrets/core/common.py +++ b/detect_secrets/core/common.py @@ -1,6 +1,11 @@ from .baseline import format_baseline_for_output -def write_baseline_to_file(filename, data): # pragma: no cover +def write_baseline_to_file(filename, data): + """ + :type filename: str + :type data: dict + :rtype: None + """ with open(filename, 'w') as f: f.write(format_baseline_for_output(data) + '\n') diff --git a/detect_secrets/main.py b/detect_secrets/main.py index cfc668934..b14ae0c9b 100644 --- a/detect_secrets/main.py +++ b/detect_secrets/main.py @@ -39,17 +39,22 @@ def main(argv=None): _scan_string(line, plugins) else: - output = baseline.format_baseline_for_output( - _perform_scan(args, plugins), + baseline_dict = _perform_scan( + args, + plugins, ) if args.import_filename: write_baseline_to_file( - args.import_filename[0], - output, + filename=args.import_filename[0], + data=baseline_dict, ) else: - print(output) + print( + baseline.format_baseline_for_output( + baseline_dict, + ), + ) elif args.action == 'audit': if not args.diff: @@ -94,6 +99,12 @@ def _scan_string(line, plugins): def _perform_scan(args, plugins): + """ + :param args: output of `argparse.ArgumentParser.parse_args` + :param plugins: tuple of initialized plugins + + :rtype: dict + """ old_baseline = _get_existing_baseline(args.import_filename) # Favors --exclude argument over existing baseline's regex (if exists) diff --git a/detect_secrets/pre_commit_hook.py b/detect_secrets/pre_commit_hook.py index 4e33fa15c..5701e15fc 100644 --- a/detect_secrets/pre_commit_hook.py +++ b/detect_secrets/pre_commit_hook.py @@ -65,8 +65,8 @@ def main(argv=None): if baseline_modified: write_baseline_to_file( - args.baseline[0], - baseline_collection.format_for_baseline_output(), + filename=args.baseline[0], + data=baseline_collection.format_for_baseline_output(), ) log.error( diff --git a/requirements-dev.txt b/requirements-dev.txt index a87d201ed..f97b0dfb4 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,3 +1,4 @@ +ipdb coverage flake8==3.5.0 mock diff --git a/tests/core/audit_test.py b/tests/core/audit_test.py index 058c3f6b5..445089a6a 100644 --- a/tests/core/audit_test.py +++ b/tests/core/audit_test.py @@ -27,7 +27,7 @@ def test_quit_before_making_decision(self, mock_printer): with self.mock_env(['q']) as m: audit.audit_baseline('will_be_mocked') - assert m.call_args[0][1] == self.baseline + assert m.call_args[1]['data'] == self.baseline assert mock_printer.message == ( 'Quitting...\n' @@ -56,7 +56,10 @@ def test_making_decisions(self, mock_printer): for secret in secrets: secret['is_secret'] = values_to_inject.pop(0) - self.run_logic(['y', 'n', 'n'], modified_baseline) + self.run_logic( + inputs=['y', 'n', 'n'], + modified_baseline=modified_baseline, + ) assert mock_printer.message == ( 'Saving progress...\n' @@ -69,7 +72,10 @@ def test_quit_half_way(self, mock_printer): secrets[0]['is_secret'] = False break - self.run_logic(['n', 'q'], modified_baseline) + self.run_logic( + inputs=['n', 'q'], + modified_baseline=modified_baseline, + ) assert mock_printer.message == ( 'Quitting...\n' @@ -86,7 +92,10 @@ def test_skip_decision(self, mock_printer): if value: secret['is_secret'] = value - self.run_logic(['s', 'y', 'y'], modified_baseline) + self.run_logic( + inputs=['s', 'y', 'y'], + modified_baseline=modified_baseline, + ) assert mock_printer.message == ( 'Saving progress...\n' @@ -102,7 +111,10 @@ def test_go_back_and_change_yes_to_no(self, mock_printer): if value is not None: secret['is_secret'] = value - self.run_logic(['s', 'y', 'b', 'n', 'y'], modified_baseline) + self.run_logic( + inputs=['s', 'y', 'b', 'n', 'y'], + modified_baseline=modified_baseline, + ) assert mock_printer.message == ( 'Saving progress...\n' @@ -118,7 +130,10 @@ def test_go_back_and_change_no_to_yes(self, mock_printer): if value is not None: secret['is_secret'] = value - self.run_logic(['s', 'n', 'b', 'y', 'y'], modified_baseline) + self.run_logic( + inputs=['s', 'n', 'b', 'y', 'y'], + modified_baseline=modified_baseline, + ) assert mock_printer.message == ( 'Saving progress...\n' @@ -134,7 +149,10 @@ def test_go_back_and_change_yes_to_skip(self, mock_printer): if value is not None: secret['is_secret'] = value - self.run_logic(['s', 'y', 'b', 's', 'y'], modified_baseline) + self.run_logic( + inputs=['s', 'y', 'b', 's', 'y'], + modified_baseline=modified_baseline, + ) assert mock_printer.message == ( 'Saving progress...\n' @@ -150,8 +168,8 @@ def test_go_back_several_steps(self, mock_printer): secret['is_secret'] = value self.run_logic( - ['s', 'y', 'b', 's', 'b', 'b', 'n', 'n', 'n'], - modified_baseline, + inputs=['s', 'y', 'b', 's', 'b', 'b', 'n', 'n', 'n'], + modified_baseline=modified_baseline, ) assert mock_printer.message == ( @@ -163,7 +181,11 @@ def test_leapfrog_decision(self, mock_printer): modified_baseline['results']['filenameA'][1]['is_secret'] = True modified_baseline['results']['filenameA'][3]['is_secret'] = True - self.run_logic(['y', 'y'], modified_baseline, self.leapfrog_baseline) + self.run_logic( + inputs=['y', 'y'], + modified_baseline=modified_baseline, + input_baseline=self.leapfrog_baseline, + ) @contextmanager def run_logic(self, inputs, modified_baseline=None, input_baseline=None): @@ -173,7 +195,7 @@ def run_logic(self, inputs, modified_baseline=None, input_baseline=None): ) as m: audit.audit_baseline('will_be_mocked') - assert m.call_args[0][1] == modified_baseline + assert m.call_args[1]['data'] == modified_baseline @contextmanager def mock_env(self, user_inputs=None, baseline=None): diff --git a/tests/main_test.py b/tests/main_test.py index aa35bab76..abce1e165 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -116,7 +116,8 @@ def test_reads_old_baseline_from_file(self, mock_merge_baseline): ) as m_write: assert main('scan --update old_baseline_file'.split()) == 0 assert m_read.call_args[0][0] == 'old_baseline_file' - assert m_write.call_args[0] == ('old_baseline_file', Any(str)) + assert m_write.call_args[1]['filename'] == 'old_baseline_file' + assert m_write.call_args[1]['data'] == Any(dict) mock_merge_baseline.assert_called_once_with( {'key': 'value'}, @@ -161,8 +162,10 @@ def test_old_baseline_ignored_with_update_flag( ), ) == 0 - assert json.loads(file_writer.call_args[0][1])['exclude_regex'] == \ + assert ( + file_writer.call_args[1]['data']['exclude_regex'] == expected_regex + ) @pytest.mark.parametrize( 'filename, expected_output', diff --git a/tests/pre_commit_hook_test.py b/tests/pre_commit_hook_test.py index da81a8666..ef795172e 100644 --- a/tests/pre_commit_hook_test.py +++ b/tests/pre_commit_hook_test.py @@ -122,7 +122,7 @@ def test_that_baseline_gets_updated( '--baseline will_be_mocked test_data/files/file_with_secrets.py', ) - baseline_written = m.call_args[0][1] + baseline_written = m.call_args[1]['data'] original_baseline = json.loads(baseline_string) assert original_baseline['exclude_regex'] == baseline_written['exclude_regex'] @@ -168,7 +168,7 @@ def test_writes_new_baseline_if_modified(self): '--baseline will_be_mocked test_data/files/file_with_secrets.py', ) - baseline_written = m.call_args[0][1] + baseline_written = m.call_args[1]['data'] original_baseline = json.loads(baseline_string) assert original_baseline['exclude_regex'] == baseline_written['exclude_regex']