Merge pull request #4440 from Flamefire/error-on-multip-pr-opts

show error when multiple PR options are passed

Author: branfosj

easybuild/main.py

@@ -333,11 +333,23 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session
 
     categorized_paths = categorize_files_by_type(eb_args)
 
+    set_pr_options = [opt for opt in (
+        'new_branch_github',
+        'new_pr',
+        'new_pr_from_branch',
+        'preview_pr',
+        'sync_branch_with_develop',
+        'sync_pr_with_develop',
+        'update_branch_github',
+        'update_pr',
+        ) if getattr(options, opt)
+    ]
+    any_pr_option_set = len(set_pr_options) > 0
+    if len(set_pr_options) > 1:
+        raise EasyBuildError("The following options are set but incompatible: %s.\nYou can only use one at a time!",
+                             ', '.join(['--' + opt.replace('_', '-') for opt in set_pr_options]))
     # command line options that do not require any easyconfigs to be specified
-    pr_options = options.new_branch_github or options.new_pr or options.new_pr_from_branch or options.preview_pr
-    pr_options = pr_options or options.sync_branch_with_develop or options.sync_pr_with_develop
-    pr_options = pr_options or options.update_branch_github or options.update_pr
-    no_ec_opts = [options.aggregate_regtest, options.regtest, pr_options, search_query]
+    no_ec_opts = [options.aggregate_regtest, options.regtest, any_pr_option_set, search_query]
 
     # determine paths to easyconfigs
     determined_paths = det_easyconfig_paths(categorized_paths['easyconfigs'])
@@ -427,9 +439,10 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session
     forced = options.force or options.rebuild
     dry_run_mode = options.dry_run or options.dry_run_short or options.missing_modules
 
-    keep_available_modules = forced or dry_run_mode or options.extended_dry_run or pr_options or options.copy_ec
-    keep_available_modules = keep_available_modules or options.inject_checksums or options.sanity_check_only
-    keep_available_modules = keep_available_modules or options.inject_checksums_to_json
+    keep_available_modules = any((
+        forced, dry_run_mode, options.extended_dry_run, any_pr_option_set, options.copy_ec, options.inject_checksums,
+        options.sanity_check_only, options.inject_checksums_to_json)
+    )
 
     # skip modules that are already installed unless forced, or unless an option is used that warrants not skipping
     if not keep_available_modules:
@@ -448,12 +461,12 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session
     if len(easyconfigs) > 0:
         # resolve dependencies if robot is enabled, except in dry run mode
         # one exception: deps *are* resolved with --new-pr or --update-pr when dry run mode is enabled
-        if options.robot and (not dry_run_mode or pr_options):
+        if options.robot and (not dry_run_mode or any_pr_option_set):
             print_msg("resolving dependencies ...", log=_log, silent=testing)
             ordered_ecs = resolve_dependencies(easyconfigs, modtool)
         else:
             ordered_ecs = easyconfigs
-    elif pr_options:
+    elif any_pr_option_set:
         ordered_ecs = None
     else:
         print_msg("No easyconfigs left to be built.", log=_log, silent=testing)
@@ -472,7 +485,7 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session
         return True
 
     # creating/updating PRs
-    if pr_options:
+    if any_pr_option_set:
         if options.new_pr:
             new_pr(categorized_paths, ordered_ecs)
         elif options.new_branch_github:

test/framework/options.py

@@ -3915,6 +3915,18 @@ def test_github_review_pr(self):
         self.mock_stderr(False)
         self.assertNotIn("2016.04", txt)
 
+    def test_set_multiple_pr_opts(self):
+        """Test that passing multiple PR options results in an error"""
+        test_cases = [
+            ['--new-pr', 'dummy.eb', '--preview-pr'],
+            ['--new-pr', 'dummy.eb', '--update-pr', '42'],
+            ['--new-pr', 'dummy.eb', '--sync-pr-with-develop', '42'],
+            ['--new-pr', 'dummy.eb', '--new-pr-from-branch', 'mybranch'],
+        ]
+        for args in test_cases:
+            error_pattern = "The following options are set but incompatible.* " + args[0]
+            self.assertErrorRegex(EasyBuildError, error_pattern, self._run_mock_eb, args, raise_error=True)
+
     def test_set_tmpdir(self):
         """Test set_tmpdir config function."""
         self.purge_environment()