Skip to content

Commit cca48a5

Browse files
stephenfinstephenfin
committed
Correct check for legacy file
A lack of an options section does not necessarily imply a legacy file: it could also indicate an incomplete "modern" file. Signed-off-by: Stephen Finucane <[email protected]>
1 parent d0dda5e commit cca48a5

File tree

2 files changed

+60
-9
lines changed

2 files changed

+60
-9
lines changed

‎pwclient/shell.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,25 @@ def main(argv=sys.argv[1:]):
3838

3939
action = args.subcmd
4040

41+
if not os.path.exists(CONFIG_FILE):
42+
sys.stderr.write('Config file not found at %s.\n' % CONFIG_FILE)
43+
sys.exit(1)
44+
4145
# grab settings from config files
4246
config = configparser.ConfigParser()
4347
config.read([CONFIG_FILE])
4448

45-
if not config.has_section('options') and os.path.exists(CONFIG_FILE):
49+
if config.has_section('base'):
4650
utils.migrate_old_config_file(CONFIG_FILE, config)
4751
sys.exit(1)
4852

53+
if not config.has_section('options'):
54+
sys.stderr.write(
55+
'No options section in %s. Did you forget to uncomment it?\n'
56+
% CONFIG_FILE
57+
)
58+
sys.exit(1)
59+
4960
if 'project' in args and args.project:
5061
project_str = args.project
5162
else:

‎tests/test_shell.py‎

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ def test_help(capsys):
9090

9191

9292
@mock.patch.object(utils.configparser, 'ConfigParser')
93+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
9394
def test_no_project(mock_config, capsys):
9495
fake_config = FakeConfig()
9596
del fake_config._data['options']['default']
@@ -106,6 +107,7 @@ def test_no_project(mock_config, capsys):
106107

107108

108109
@mock.patch.object(utils.configparser, 'ConfigParser')
110+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
109111
def test_no_project_url(mock_config, capsys):
110112
fake_config = FakeConfig()
111113
del fake_config._data[DEFAULT_PROJECT]['url']
@@ -122,6 +124,7 @@ def test_no_project_url(mock_config, capsys):
122124

123125

124126
@mock.patch.object(utils.configparser, 'ConfigParser')
127+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
125128
def test_missing_project(mock_config, capsys):
126129
mock_config.return_value = FakeConfig()
127130

@@ -160,6 +163,7 @@ def test_migrate_config(mock_migrate, mock_config):
160163

161164

162165
@mock.patch.object(utils.configparser, 'ConfigParser')
166+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
163167
@mock.patch.object(api, 'XMLRPC')
164168
@mock.patch.object(patches, 'action_apply')
165169
def test_server_error(mock_action, mock_api, mock_config, capsys):
@@ -176,6 +180,7 @@ def test_server_error(mock_action, mock_api, mock_config, capsys):
176180

177181

178182
@mock.patch.object(utils.configparser, 'ConfigParser')
183+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
179184
@mock.patch.object(api, 'XMLRPC')
180185
@mock.patch.object(patches, 'action_apply')
181186
def test_apply(mock_action, mock_api, mock_config):
@@ -203,6 +208,7 @@ def test_apply(mock_action, mock_api, mock_config):
203208

204209

205210
@mock.patch.object(utils.configparser, 'ConfigParser')
211+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
206212
@mock.patch.object(api, 'XMLRPC')
207213
@mock.patch.object(patches, 'action_apply')
208214
def test_apply__failed(mock_action, mock_api, mock_config, capsys):
@@ -225,6 +231,7 @@ def test_apply__failed(mock_action, mock_api, mock_config, capsys):
225231

226232

227233
@mock.patch.object(utils.configparser, 'ConfigParser')
234+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
228235
@mock.patch.object(api, 'XMLRPC')
229236
@mock.patch.object(checks, 'action_create')
230237
def test_check_create(mock_action, mock_api, mock_config):
@@ -263,6 +270,7 @@ def test_check_create(mock_action, mock_api, mock_config):
263270

264271

265272
@mock.patch.object(utils.configparser, 'ConfigParser')
273+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
266274
@mock.patch.object(api, 'XMLRPC')
267275
@mock.patch.object(checks, 'action_create')
268276
def test_check_create__no_auth(
@@ -296,6 +304,7 @@ def test_check_create__no_auth(
296304

297305

298306
@mock.patch.object(utils.configparser, 'ConfigParser')
307+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
299308
@mock.patch.object(api, 'XMLRPC')
300309
@mock.patch.object(checks, 'action_info')
301310
def test_check_info(mock_action, mock_api, mock_config):
@@ -307,6 +316,7 @@ def test_check_info(mock_action, mock_api, mock_config):
307316

308317

309318
@mock.patch.object(utils.configparser, 'ConfigParser')
319+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
310320
@mock.patch.object(api, 'XMLRPC')
311321
@mock.patch.object(checks, 'action_info')
312322
def test_check_info__no_patch_id(mock_action, mock_api, mock_config):
@@ -318,6 +328,7 @@ def test_check_info__no_patch_id(mock_action, mock_api, mock_config):
318328

319329

320330
@mock.patch.object(utils.configparser, 'ConfigParser')
331+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
321332
@mock.patch.object(api, 'XMLRPC')
322333
@mock.patch.object(checks, 'action_list')
323334
def test_check_list(mock_action, mock_api, mock_config):
@@ -329,6 +340,7 @@ def test_check_list(mock_action, mock_api, mock_config):
329340

330341

331342
@mock.patch.object(utils.configparser, 'ConfigParser')
343+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
332344
@mock.patch.object(api, 'XMLRPC')
333345
@mock.patch.object(patches, 'action_get')
334346
def test_get__numeric_id(mock_action, mock_api, mock_config):
@@ -341,6 +353,7 @@ def test_get__numeric_id(mock_action, mock_api, mock_config):
341353

342354

343355
@mock.patch.object(utils.configparser, 'ConfigParser')
356+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
344357
@mock.patch.object(api, 'XMLRPC')
345358
@mock.patch.object(patches, 'action_get')
346359
def test_get__multiple_ids(mock_action, mock_api, mock_config):
@@ -359,6 +372,7 @@ def test_get__multiple_ids(mock_action, mock_api, mock_config):
359372

360373

361374
@mock.patch.object(utils.configparser, 'ConfigParser')
375+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
362376
@mock.patch.object(api, 'XMLRPC')
363377
@mock.patch.object(patches, 'patch_id_from_hash')
364378
@mock.patch.object(patches, 'action_get')
@@ -376,6 +390,7 @@ def test_get__hash_ids(mock_action, mock_hash, mock_api, mock_config):
376390

377391

378392
@mock.patch.object(utils.configparser, 'ConfigParser')
393+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
379394
@mock.patch.object(api, 'XMLRPC')
380395
@mock.patch.object(patches, 'action_get')
381396
def test_get__no_ids(mock_action, mock_api, mock_config, capsys):
@@ -392,6 +407,7 @@ def test_get__no_ids(mock_action, mock_api, mock_config, capsys):
392407

393408

394409
@mock.patch.object(utils.configparser, 'ConfigParser')
410+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
395411
@mock.patch.object(api, 'XMLRPC')
396412
@mock.patch.object(patches, 'action_apply')
397413
def test_git_am__no_args(mock_action, mock_api, mock_config):
@@ -421,6 +437,7 @@ def test_git_am__no_args(mock_action, mock_api, mock_config):
421437

422438

423439
@mock.patch.object(utils.configparser, 'ConfigParser')
440+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
424441
@mock.patch.object(api, 'XMLRPC')
425442
@mock.patch.object(patches, 'action_apply')
426443
def test_git_am__threeway_option(mock_action, mock_api, mock_config):
@@ -435,6 +452,7 @@ def test_git_am__threeway_option(mock_action, mock_api, mock_config):
435452

436453

437454
@mock.patch.object(utils.configparser, 'ConfigParser')
455+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
438456
@mock.patch.object(api, 'XMLRPC')
439457
@mock.patch.object(patches, 'action_apply')
440458
def test_git_am__signoff_option(mock_action, mock_api, mock_config):
@@ -450,6 +468,7 @@ def test_git_am__signoff_option(mock_action, mock_api, mock_config):
450468

451469

452470
@mock.patch.object(utils.configparser, 'ConfigParser')
471+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
453472
@mock.patch.object(api, 'XMLRPC')
454473
@mock.patch.object(patches, 'action_apply')
455474
def test_git_am__threeway_global_conf(mock_action, mock_api, mock_config):
@@ -470,6 +489,7 @@ def test_git_am__threeway_global_conf(mock_action, mock_api, mock_config):
470489

471490

472491
@mock.patch.object(utils.configparser, 'ConfigParser')
492+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
473493
@mock.patch.object(api, 'XMLRPC')
474494
@mock.patch.object(patches, 'action_apply')
475495
def test_git_am__signoff_global_conf(mock_action, mock_api, mock_config):
@@ -491,6 +511,7 @@ def test_git_am__signoff_global_conf(mock_action, mock_api, mock_config):
491511

492512

493513
@mock.patch.object(utils.configparser, 'ConfigParser')
514+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
494515
@mock.patch.object(api, 'XMLRPC')
495516
@mock.patch.object(patches, 'action_apply')
496517
def test_git_am__threeway_project_conf(mock_action, mock_api, mock_config):
@@ -511,6 +532,7 @@ def test_git_am__threeway_project_conf(mock_action, mock_api, mock_config):
511532

512533

513534
@mock.patch.object(utils.configparser, 'ConfigParser')
535+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
514536
@mock.patch.object(api, 'XMLRPC')
515537
@mock.patch.object(patches, 'action_apply')
516538
def test_git_am__signoff_project_conf(mock_action, mock_api, mock_config):
@@ -532,6 +554,7 @@ def test_git_am__signoff_project_conf(mock_action, mock_api, mock_config):
532554

533555

534556
@mock.patch.object(utils.configparser, 'ConfigParser')
557+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
535558
@mock.patch.object(api, 'XMLRPC')
536559
@mock.patch.object(patches, 'action_apply')
537560
def test_git_am__failure(mock_action, mock_api, mock_config, capsys):
@@ -553,6 +576,7 @@ def test_git_am__failure(mock_action, mock_api, mock_config, capsys):
553576

554577

555578
@mock.patch.object(utils.configparser, 'ConfigParser')
579+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
556580
@mock.patch.object(api, 'XMLRPC')
557581
@mock.patch.object(patches, 'action_info')
558582
def test_info(mock_action, mock_api, mock_config):
@@ -580,6 +604,7 @@ def test_info(mock_action, mock_api, mock_config):
580604

581605

582606
@mock.patch.object(utils.configparser, 'ConfigParser')
607+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
583608
@mock.patch.object(api, 'XMLRPC')
584609
@mock.patch.object(patches, 'action_list')
585610
def test_list__no_options(mock_action, mock_api, mock_config):
@@ -603,6 +628,7 @@ def test_list__no_options(mock_action, mock_api, mock_config):
603628

604629

605630
@mock.patch.object(utils.configparser, 'ConfigParser')
631+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
606632
@mock.patch.object(api, 'XMLRPC')
607633
@mock.patch.object(patches, 'action_list')
608634
def test_list__state_filter(mock_action, mock_api, mock_config):
@@ -626,6 +652,7 @@ def test_list__state_filter(mock_action, mock_api, mock_config):
626652

627653

628654
@mock.patch.object(utils.configparser, 'ConfigParser')
655+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
629656
@mock.patch.object(api, 'XMLRPC')
630657
@mock.patch.object(patches, 'action_list')
631658
def test_list__archived_filter(mock_action, mock_api, mock_config):
@@ -649,6 +676,7 @@ def test_list__archived_filter(mock_action, mock_api, mock_config):
649676

650677

651678
@mock.patch.object(utils.configparser, 'ConfigParser')
679+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
652680
@mock.patch.object(api, 'XMLRPC')
653681
@mock.patch.object(patches, 'action_list')
654682
def test_list__project_filter(mock_action, mock_api, mock_config):
@@ -678,6 +706,7 @@ def test_list__project_filter(mock_action, mock_api, mock_config):
678706

679707

680708
@mock.patch.object(utils.configparser, 'ConfigParser')
709+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
681710
@mock.patch.object(api, 'XMLRPC')
682711
@mock.patch.object(patches, 'action_list')
683712
def test_list__submitter_filter(mock_action, mock_api, mock_config):
@@ -701,6 +730,7 @@ def test_list__submitter_filter(mock_action, mock_api, mock_config):
701730

702731

703732
@mock.patch.object(utils.configparser, 'ConfigParser')
733+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
704734
@mock.patch.object(api, 'XMLRPC')
705735
@mock.patch.object(patches, 'action_list')
706736
def test_list__delegate_filter(mock_action, mock_api, mock_config):
@@ -724,6 +754,7 @@ def test_list__delegate_filter(mock_action, mock_api, mock_config):
724754

725755

726756
@mock.patch.object(utils.configparser, 'ConfigParser')
757+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
727758
@mock.patch.object(api, 'XMLRPC')
728759
@mock.patch.object(patches, 'action_list')
729760
def test_list__msgid_filter(mock_action, mock_api, mock_config):
@@ -747,6 +778,7 @@ def test_list__msgid_filter(mock_action, mock_api, mock_config):
747778

748779

749780
@mock.patch.object(utils.configparser, 'ConfigParser')
781+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
750782
@mock.patch.object(api, 'XMLRPC')
751783
@mock.patch.object(patches, 'action_list')
752784
def test_list__name_filter(mock_action, mock_api, mock_config):
@@ -770,6 +802,7 @@ def test_list__name_filter(mock_action, mock_api, mock_config):
770802

771803

772804
@mock.patch.object(utils.configparser, 'ConfigParser')
805+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
773806
@mock.patch.object(api, 'XMLRPC')
774807
@mock.patch.object(patches, 'action_list')
775808
def test_list__limit_filter(mock_action, mock_api, mock_config):
@@ -793,6 +826,7 @@ def test_list__limit_filter(mock_action, mock_api, mock_config):
793826

794827

795828
@mock.patch.object(utils.configparser, 'ConfigParser')
829+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
796830
@mock.patch.object(api, 'XMLRPC')
797831
@mock.patch.object(patches, 'action_list')
798832
def test_list__limit_reverse_filter(mock_action, mock_api, mock_config):
@@ -816,6 +850,7 @@ def test_list__limit_reverse_filter(mock_action, mock_api, mock_config):
816850

817851

818852
@mock.patch.object(utils.configparser, 'ConfigParser')
853+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
819854
@mock.patch.object(api, 'XMLRPC')
820855
@mock.patch.object(patches, 'action_list')
821856
def test_list__hash_filter(mock_action, mock_api, mock_config):
@@ -839,6 +874,7 @@ def test_list__hash_filter(mock_action, mock_api, mock_config):
839874

840875

841876
@mock.patch.object(utils.configparser, 'ConfigParser')
877+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
842878
@mock.patch.object(api, 'XMLRPC')
843879
@mock.patch.object(projects, 'action_list')
844880
def test_projects(mock_action, mock_api, mock_config):
@@ -850,6 +886,7 @@ def test_projects(mock_action, mock_api, mock_config):
850886

851887

852888
@mock.patch.object(utils.configparser, 'ConfigParser')
889+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
853890
@mock.patch.object(api, 'XMLRPC')
854891
@mock.patch.object(states, 'action_list')
855892
def test_states(mock_action, mock_api, mock_config):
@@ -861,6 +898,7 @@ def test_states(mock_action, mock_api, mock_config):
861898

862899

863900
@mock.patch.object(utils.configparser, 'ConfigParser')
901+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
864902
@mock.patch.object(api, 'XMLRPC')
865903
@mock.patch.object(patches, 'action_update')
866904
def test_update__no_options(
@@ -888,6 +926,7 @@ def test_update__no_options(
888926

889927

890928
@mock.patch.object(utils.configparser, 'ConfigParser')
929+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
891930
@mock.patch.object(api, 'XMLRPC')
892931
@mock.patch.object(patches, 'action_update')
893932
def test_update__no_auth(
@@ -908,6 +947,7 @@ def test_update__no_auth(
908947

909948

910949
@mock.patch.object(utils.configparser, 'ConfigParser')
950+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
911951
@mock.patch.object(api, 'XMLRPC')
912952
@mock.patch.object(patches, 'action_update')
913953
def test_update__state_option(mock_action, mock_api, mock_config):
@@ -932,6 +972,7 @@ def test_update__state_option(mock_action, mock_api, mock_config):
932972

933973

934974
@mock.patch.object(utils.configparser, 'ConfigParser')
975+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
935976
@mock.patch.object(api, 'XMLRPC')
936977
@mock.patch.object(patches, 'action_update')
937978
def test_update__archive_option(mock_action, mock_api, mock_config):
@@ -952,6 +993,7 @@ def test_update__archive_option(mock_action, mock_api, mock_config):
952993

953994

954995
@mock.patch.object(utils.configparser, 'ConfigParser')
996+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
955997
@mock.patch.object(api, 'XMLRPC')
956998
@mock.patch.object(patches, 'action_update')
957999
def test_update__commitref_option(mock_action, mock_api, mock_config):
@@ -976,6 +1018,7 @@ def test_update__commitref_option(mock_action, mock_api, mock_config):
9761018

9771019

9781020
@mock.patch.object(utils.configparser, 'ConfigParser')
1021+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
9791022
@mock.patch.object(api, 'XMLRPC')
9801023
@mock.patch.object(patches, 'action_update')
9811024
def test_update__commitref_with_multiple_patches(
@@ -1002,17 +1045,14 @@ def test_update__commitref_with_multiple_patches(
10021045
assert 'Declining update with COMMIT-REF on multiple IDs' in captured.err
10031046

10041047

1005-
@mock.patch.object(patches, 'action_view')
10061048
@mock.patch.object(utils.configparser, 'ConfigParser')
1049+
@mock.patch.object(shell.os.path, 'exists', new=mock.Mock(return_value=True))
10071050
@mock.patch.object(api, 'XMLRPC')
1008-
def test_view(mock_api, mock_config, mock_view, capsys):
1009-
fake_config = FakeConfig()
1010-
1011-
mock_config.return_value = fake_config
1051+
@mock.patch.object(patches, 'action_view')
1052+
def test_view(mock_action, mock_api, mock_config, capsys):
1053+
mock_config.return_value = FakeConfig()
10121054
mock_api.return_value.patch_get_mbox.return_value = 'foo'
10131055

1014-
# test firstly with a single patch ID
1015-
10161056
shell.main(['view', '1'])
10171057

1018-
mock_view.assert_called_once_with(mock_api.return_value, [1])
1058+
mock_action.assert_called_once_with(mock_api.return_value, [1])

0 commit comments

Comments
 (0)