simplify vendor commit checker

openshift · Jul 11, 2018 · a9b4aee · a9b4aee
1 parent b10515a
commit a9b4aee
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 28 deletions.
diff --git a/tools/rebasehelpers/commitchecker/validate.go b/tools/rebasehelpers/commitchecker/validate.go
@@ -18,11 +18,11 @@ The following UPSTREAM commits have invalid summaries:
 {{ end }}
 UPSTREAM commit summaries should look like:
 
-  UPSTREAM: [non-kube-repo/name: ]<PR number|carry|drop>: description
+  UPSTREAM: <PR number|carry|drop>: description
 
 UPSTREAM commits which revert previous UPSTREAM commits should look like:
 
-  UPSTREAM: revert: <sha>: <normal upstream format>
+  UPSTREAM: revert: <normal upstream format>
 
 UPSTREAM commits are validated against the following regular expression:
 
@@ -31,20 +31,16 @@ UPSTREAM commits are validated against the following regular expression:
 Examples of valid summaries:
 
   UPSTREAM: 12345: A kube fix
-  UPSTREAM: coreos/etcd: 12345: An etcd fix
   UPSTREAM: <carry>: A carried kube change
   UPSTREAM: <drop>: A dropped kube change
-  UPSTREAM: revert: abcd123: coreos/etcd: 12345: An etcd fix
-  UPSTREAM: k8s.io/heapster: 12345: A heapster fix
+  UPSTREAM: revert: 12345: A kube revert
 
 `
 
 var AllValidators = []func([]util.Commit) error{
 	ValidateUpstreamCommitSummaries,
 	ValidateUpstreamCommitsWithoutGodepsChanges,
-	ValidateUpstreamCommitModifiesSingleGodepsRepo,
 	ValidateUpstreamCommitModifiesOnlyGodeps,
-	ValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo,
 	ValidateUpstreamCommitModifiesOnlyKubernetes,
 }
 

diff --git a/tools/rebasehelpers/commitchecker/validate_test.go b/tools/rebasehelpers/commitchecker/validate_test.go
@@ -296,29 +296,31 @@ func TestValidateUpstreamCommitSummaries(t *testing.T) {
 		{valid: true, summary: "UPSTREAM: <drop>: a change"},
 		{valid: true, summary: "UPSTREAM: coreos/etcd: <carry>: a change"},
 		{valid: true, summary: "UPSTREAM: coreos/etcd: <drop>: a change"},
-		{valid: true, summary: "UPSTREAM: revert: abcd123: 12345: a change"},
-		{valid: true, summary: "UPSTREAM: revert: abcd123: k8s.io/heapster: 12345: a change"},
-		{valid: true, summary: "UPSTREAM: revert: abcd123: <carry>: a change"},
-		{valid: true, summary: "UPSTREAM: revert: abcd123: <drop>: a change"},
-		{valid: true, summary: "UPSTREAM: revert: abcd123: coreos/etcd: <carry>: a change"},
-		{valid: true, summary: "UPSTREAM: revert: abcd123: coreos/etcd: <drop>: a change"},
+		{valid: true, summary: "UPSTREAM: revert: 12345: a change"},
+		{valid: true, summary: "UPSTREAM: revert: k8s.io/heapster: 12345: a change"},
+		{valid: true, summary: "UPSTREAM: revert: <carry>: a change"},
+		{valid: true, summary: "UPSTREAM: revert: <drop>: a change"},
+		{valid: true, summary: "UPSTREAM: revert: coreos/etcd: <carry>: a change"},
+		{valid: true, summary: "UPSTREAM: revert: coreos/etcd: <drop>: a change"},
 		{valid: false, summary: "UPSTREAM: whoopsie daisy"},
 		{valid: true, summary: "UPSTREAM: gopkg.in/ldap.v2: 51: exposed better API for paged search"},
 	}
 	for _, test := range tests {
-		commit := util.Commit{Summary: test.summary, Sha: "abcd000"}
-		err := ValidateUpstreamCommitSummaries([]util.Commit{commit})
-		if err != nil {
-			if test.valid {
-				t.Fatalf("unexpected error:\n%s", err)
+		t.Run(test.summary, func(t *testing.T) {
+			commit := util.Commit{Summary: test.summary, Sha: "abcd000"}
+			err := ValidateUpstreamCommitSummaries([]util.Commit{commit})
+			if err != nil {
+				if test.valid {
+					t.Fatalf("unexpected error:\n%s", err)
+				} else {
+					t.Logf("got expected error:\n%s", err)
+				}
 			} else {
-				t.Logf("got expected error:\n%s", err)
+				if !test.valid {
+					t.Fatalf("expected an error, got none; summary: %s", test.summary)
+				}
 			}
-		} else {
-			if !test.valid {
-				t.Fatalf("expected an error, got none; summary: %s", test.summary)
-			}
-		}
+		})
 	}
 }
 
@@ -409,7 +411,7 @@ func TestValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo(t *testing.T) {
 			commits: []util.Commit{
 				{
 					Sha:     "aaa0001",
-					Summary: "UPSTREAM: revert: abcd000: 12345: a change",
+					Summary: "UPSTREAM: revert: 12345: a change",
 					Files: []util.File{
 						"Godeps/_workspace/src/k8s.io/kubernetes/file1",
 						"Godeps/_workspace/src/k8s.io/kubernetes/file2",
@@ -423,7 +425,7 @@ func TestValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo(t *testing.T) {
 			commits: []util.Commit{
 				{
 					Sha:     "aaa0001",
-					Summary: "UPSTREAM: revert: abcd000: coreos/etcd: 12345: a change",
+					Summary: "UPSTREAM: revert: coreos/etcd: 12345: a change",
 					Files: []util.File{
 						"Godeps/_workspace/src/k8s.io/kubernetes/file1",
 						"Godeps/_workspace/src/k8s.io/kubernetes/file2",
@@ -437,7 +439,7 @@ func TestValidateUpstreamCommitModifiesOnlyDeclaredGodepRepo(t *testing.T) {
 			commits: []util.Commit{
 				{
 					Sha:     "aaa0001",
-					Summary: "UPSTREAM: revert: abcd000: coreos/etcd: 12345: a change",
+					Summary: "UPSTREAM: revert: coreos/etcd: 12345: a change",
 					Files: []util.File{
 						"Godeps/_workspace/src/github.com/coreos/etcd/file1",
 						"Godeps/_workspace/src/github.com/coreos/etcd/file2",

diff --git a/tools/rebasehelpers/util/git.go b/tools/rebasehelpers/util/git.go
@@ -9,7 +9,7 @@ import (
 	"strings"
 )
 
-var UpstreamSummaryPattern = regexp.MustCompile(`UPSTREAM: (revert: [a-f0-9]{7,}: )?(([\w\.-]+\/[\w-\.-]+)?: )?(\d+:|<carry>:|<drop>:)`)
+var UpstreamSummaryPattern = regexp.MustCompile(`UPSTREAM: (revert: )?(([\w\.-]+\/[\w-\.-]+)?: )?(\d+:|<carry>:|<drop>:)`)
 
 // supportedHosts maps source hosts to the number of path segments that
 // represent the account/repo for that host. This is necessary because we