fix checking for new easyconfigs in copy_easyconfigs, pick up specified commit message as PR title if none was provided

.travis.yml

@@ -95,5 +95,4 @@ script:
     # unset $PYTHONPATH to avoid mixing two EasyBuild 'installations' when testing bootstrapped EasyBuild module
     - unset PYTHONPATH
     # simply sanity check on bootstrapped EasyBuild module
-    # note: temporarily disabled because bootstrap test fails, cfr. https://github.com/hpcugent/easybuild-framework/issues/1869
     - module use /tmp/$TRAVIS_JOB_ID/eb_bootstrap/modules/all; module load EasyBuild; eb --version

easybuild/framework/easyconfig/easyconfig.py

@@ -1552,10 +1552,11 @@ def copy_easyconfigs(paths, target_dir):
             ec_filename = '%s-%s.eb' % (soft_name, det_full_ec_version(file_info['ecs'][-1]))
 
             target_path = det_location_for(path, target_dir, soft_name, ec_filename)
+
+            file_info['new'].append(not os.path.exists(target_path))
             copy_file(path, target_path, force_in_dry_run=True)
 
             file_info['paths_in_repo'].append(target_path)
-            file_info['new'].append(os.path.exists(target_path))
 
         else:
             raise EasyBuildError("Multiple EasyConfig instances obtained from easyconfig file %s", path)

easybuild/tools/github.py

@@ -879,7 +879,9 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None):
     class_label = ','.join([tc for (cnt, tc) in classes_counted if cnt == classes_counted[-1][0]])
 
     if title is None:
-        if file_info['ecs'] and all(file_info['new']) and not deleted_paths:
+        if commit_msg:
+            title = commit_msg
+        elif file_info['ecs'] and all(file_info['new']) and not deleted_paths:
             # mention software name/version in PR title (only first 3)
             names_and_versions = ["%s v%s" % (ec.name, ec.version) for ec in file_info['ecs']]
             if len(names_and_versions) <= 3:

test/framework/easyconfig.py

@@ -1802,13 +1802,20 @@ def test_copy_easyconfigs(self):
             ('g/GCC/GCC-4.6.3.eb', 'GCC.eb'),
             ('o/OpenMPI/OpenMPI-1.6.4-GCC-4.6.4.eb', 'openmpi164.eb'),
             ('t/toy/toy-0.0-gompi-1.3.12-test.eb', 'foo.eb'),
+            ('t/toy/toy-0.0.eb', 'TOY.eb'),
         ]
         ecs_to_copy = []
         for (src_ec, target_ec) in test_ecs:
             ecs_to_copy.append(os.path.join(self.test_prefix, target_ec))
             shutil.copy2(os.path.join(test_ecs_dir, src_ec), ecs_to_copy[-1])
 
-        copy_easyconfigs(ecs_to_copy, target_dir)
+        res = copy_easyconfigs(ecs_to_copy, target_dir)
+        self.assertEqual(sorted(res.keys()), ['ecs', 'new', 'paths_in_repo'])
+        self.assertEqual(len(res['ecs']), len(test_ecs))
+        self.assertTrue(all(isinstance(ec, EasyConfig) for ec in res['ecs']))
+        self.assertTrue(all(res['new']))
+        expected = os.path.join(target_dir, 'easybuild', 'easyconfigs', 'g', 'GCC', 'GCC-4.6.3.eb')
+        self.assertTrue(os.path.samefile(res['paths_in_repo'][0], expected))
 
         # check whether easyconfigs were copied (unmodified) to correct location
         for orig_ec, src_ec in test_ecs:
@@ -1833,8 +1840,19 @@ def test_copy_easyconfigs(self):
         write_file(toy_ec, toy_ec_txt)
 
         # verify whether copied easyconfig gets cleaned up
-        copy_easyconfigs([toy_ec], target_dir)
+        res = copy_easyconfigs([toy_ec], target_dir)
+
+        self.assertEqual([len(x) for x in res.values()], [1, 1, 1])
+
+        self.assertTrue(isinstance(res['ecs'][0], EasyConfig))
+        self.assertEqual(res['ecs'][0].full_mod_name, 'toy/0.0')
+
+        # toy-0.0.eb was already copied into target_dir, so should not be marked as new anymore
+        self.assertFalse(res['new'][0])
+
         copied_toy_ec = os.path.join(ecs_target_dir, 't', 'toy', 'toy-0.0.eb')
+        self.assertTrue(os.path.samefile(res['paths_in_repo'][0], copied_toy_ec))
+
 
         txt = read_file(copied_toy_ec)
         regexs = [

test/framework/github.py

@@ -163,9 +163,9 @@ def test_download_repo(self):
             print "Skipping test_download_repo, no GitHub token available?"
             return
 
-        # default: download tarball for master branch of hpcugent/easybuild-easyconfigs repo
+        # default: download tarball for master branch of easybuilders/easybuild-easyconfigs repo
         path = gh.download_repo(path=self.test_prefix, github_user=GITHUB_TEST_ACCOUNT)
-        repodir = os.path.join(self.test_prefix, 'hpcugent', 'easybuild-easyconfigs-master')
+        repodir = os.path.join(self.test_prefix, 'easybuilders', 'easybuild-easyconfigs-master')
         self.assertTrue(os.path.samefile(path, repodir))
         self.assertTrue(os.path.exists(repodir))
         shafile = os.path.join(repodir, 'latest-sha')

test/framework/options.py

@@ -2463,13 +2463,15 @@ def test_new_update_pr(self):
         self.mock_stdout(False)
 
         # add required commit message, try again
-        args.append('--pr-commit-msg="just a test"')
+        args.append('--pr-commit-msg=just a test')
         self.mock_stdout(True)
         self.eb_main(args, do_build=True, raise_error=True, testing=False)
         txt = self.get_stdout()
         self.mock_stdout(False)
 
         regexs[-1] = r"^\s*2 files changed"
+        regexs.remove(r"^\* title: \"\{tools\}\[gompi/1.3.12\] toy v0.0\"")
+        regexs.append(r"^\* title: \"just a test\"")
         regexs.append(r".*/toy-0.0_typo.patch\s*\|")
         for regex in regexs:
             regex = re.compile(regex, re.M)
@@ -2508,6 +2510,32 @@ def test_new_update_pr(self):
             regex = re.compile(regex, re.M)
             self.assertTrue(regex.search(txt), "Pattern '%s' found in: %s" % (regex.pattern, txt))
 
+        # modifying an existing easyconfig requires a custom PR title
+        gcc_ec = os.path.join(test_ecs, 'g', 'GCC', 'GCC-4.9.2.eb')
+        self.assertTrue(os.path.exists(gcc_ec))
+
+        args = [
+            '--new-pr',
+            '--github-user=%s' % GITHUB_TEST_ACCOUNT,
+            toy_ec,
+            gcc_ec,
+            '-D',
+        ]
+        error_msg = "A meaningful commit message must be specified via --pr-commit-msg"
+        self.mock_stdout(True)
+        self.assertErrorRegex(EasyBuildError, error_msg, self.eb_main, args, raise_error=True)
+        self.mock_stdout(False)
+
+        # also specifying commit message is sufficient; PR title is inherited from commit message
+        args.append('--pr-commit-msg=this is just a test')
+        self.mock_stdout(True)
+        self.eb_main(args, do_build=True, raise_error=True, testing=False)
+        txt = self.get_stdout()
+        self.mock_stdout(False)
+
+        regex = re.compile('^\* title: "this is just a test"', re.M)
+        self.assertTrue(regex.search(txt), "Pattern '%s' is found in: %s" % (regex.pattern, txt))
+
         args = [
             # PR for EasyBuild v2.5.0 release
             # we need a PR where the base branch is still available ('develop', in this case)
@@ -2672,6 +2700,7 @@ def test_empty_pr(self):
             ec,
             '-D',
             '--github-user=%s' % GITHUB_TEST_ACCOUNT,
+            '--pr-commit-msg=blabla',
         ]
 
         self.mock_stdout(True)