Fix version qualifier manipulation logic

pomupdate/buildpomupdate.py

@@ -236,12 +236,21 @@ def _sanitize_version_qualifier(version_qualifier):
 
 
 def _append_version_qualifier(current_version, version_qualifier):
-    return "%s-%s" % (current_version, version_qualifier)
+    if current_version.endswith(version_qualifier):
+        # we won't re-append the same qualifier ...
+        return current_version
+    else:
+        return "%s-%s" % (current_version, version_qualifier)
 
 
-def _insert_version_qualifier(version, version_qualifier):
-    i = version.rfind("-")
-    return "%s-%s-%s" % (version[0:i], version_qualifier, version[i+1:])
+def _insert_version_qualifier(current_version, version_qualifier):
+    if current_version.endswith(version_qualifier):
+        # we won't insert the same qualifier
+        return current_version
+    else:
+        i = current_version.rfind("-")
+        return "%s-%s-%s" % (current_version[0:i], version_qualifier,
+                             current_version[i+1:])
 
 
 def _remove_version_qualifier(current_version, version_qualifier):
@@ -252,8 +261,12 @@ def _remove_version_qualifier(current_version, version_qualifier):
     i = current_version.find(version_qualifier)
     if i == -1:
         return current_version
+    # current_version: abc-rel9
+    # version_qualifier: -rel
+    # i = 3
+    # end_index = 3 + 4 = 7
     end_index = i + len(version_qualifier)
-    if end_index == len(current_version) - 1 or current_version[end_index-1] == "-":
+    if end_index == len(current_version) or current_version[end_index-1] == "-":
         # the given version_qualifier matches a vq in the current version
         pass
     else:

tests/buildpomupdatetest.py

@@ -535,8 +535,6 @@ def test_update_version_in_BUILD_pom__rm_version_qualifier__substr1(self):
         self._write_build_pom(pack1_path, "p1a", "p1g", "3.2.1-SNAPSHOT",
                               version_increment_strategy="major")
 
-        # not actually removed, since "SNAP" is a substring, but not a full
-        # version qualifier
         buildpomupdate.update_build_pom_file(
             repo_root, [pack1], version_qualifier_to_remove="SNAP")
 
@@ -556,8 +554,6 @@ def test_update_version_in_BUILD_pom__rm_version_qualifier__substr2(self):
         self._write_build_pom(pack1_path, "p1a", "p1g", "3.2.1-foo-blah",
                               version_increment_strategy="major")
 
-        # not actually removed, since "SNAP" is a substring, but not a full
-        # version qualifier
         buildpomupdate.update_build_pom_file(
             repo_root, [pack1], version_qualifier_to_remove="fo")
 
@@ -574,13 +570,11 @@ def test_update_version_in_BUILD_pom__rm_version_qualifier__substr3(self):
         repo_root = tempfile.mkdtemp("monorepo")
         pack1_path = os.path.join(repo_root, pack1)
         os.makedirs(pack1_path)
-        self._write_build_pom(pack1_path, "p1a", "p1g", "3.2.1-foo-blah",
+        self._write_build_pom(pack1_path, "p1a", "p1g", "3.2.1-foo-rel9",
                               version_increment_strategy="major")
 
-        # not actually removed, since "SNAP" is a substring, but not a full
-        # version qualifier
         buildpomupdate.update_build_pom_file(
-            repo_root, [pack1], version_qualifier_to_remove="bl")
+            repo_root, [pack1], version_qualifier_to_remove="rel")
 
         with open(os.path.join(pack1_path, "MVN-INF", "BUILD.pom"), "r") as f:
             content = f.read()
@@ -630,6 +624,46 @@ def test_update_version_in_BUILD_pom__add_version_qualifier__non_snapshot_qualif
             self.assertIn('version = "3.2.1-rel1-rel2"', content)
             self.assertIn(')', content)
 
+    def test_update_version_in_BUILD_pom__add_version_qualifier__duplicate_is_not_repeated(self):
+        pack1 = "somedir/p1"
+        repo_root = tempfile.mkdtemp("monorepo")
+        pack1_path = os.path.join(repo_root, pack1)
+        os.makedirs(pack1_path)
+        self._write_build_pom(pack1_path, "p1a", "p1g", "3.2.1-casino",
+                              version_increment_strategy="major")
+
+        buildpomupdate.update_build_pom_file(
+            repo_root, [pack1], version_qualifier_to_add="casino")
+
+        with open(os.path.join(pack1_path, "MVN-INF", "BUILD.pom"), "r") as f:
+            content = f.read()
+            self.assertIn('maven_artifact(', content)
+            self.assertIn('group_id = "p1g"', content)
+            self.assertIn('artifact_id = "p1a"', content)
+            # -casino is not appended if the version ends with -casino already
+            self.assertIn('version = "3.2.1-casino"', content)
+            self.assertIn(')', content)
+
+    def test_update_version_in_BUILD_pom__add_version_qualifier__no_duplicate_SNAPSHOT(self):
+        pack1 = "somedir/p1"
+        repo_root = tempfile.mkdtemp("monorepo")
+        pack1_path = os.path.join(repo_root, pack1)
+        os.makedirs(pack1_path)
+        self._write_build_pom(pack1_path, "p1a", "p1g", "3.2.1-SNAPSHOT",
+                              version_increment_strategy="major")
+
+        buildpomupdate.update_build_pom_file(
+            repo_root, [pack1], version_qualifier_to_add="SNAPSHOT")
+
+        with open(os.path.join(pack1_path, "MVN-INF", "BUILD.pom"), "r") as f:
+            content = f.read()
+            self.assertIn('maven_artifact(', content)
+            self.assertIn('group_id = "p1g"', content)
+            self.assertIn('artifact_id = "p1a"', content)
+            # -SNAPSHOT is not added if the version ends with -SNAPSHOT already
+            self.assertIn('version = "3.2.1-SNAPSHOT"', content)
+            self.assertIn(')', content)
+
     def test_update_pom_generation_mode_in_BUILD_pom(self):
         pack1 = "somedir/p1"
         repo_root = tempfile.mkdtemp("monorepo")