Repository: osbuild/osbuild-composer
Pull request #897 (2020-07-29 17:39:22)

store: Fix copy failure when initializing blueprints commit list
copy() will not append to a slice, so if the length is 0 nothing will be
copied. Initialize the slice with the length instead of the capacity.

This fixes a problem with blueprint commits vanishing when the server is
restarted.

ondrejbudai: Can we cover this by a test-case?
bcl: > Can we cover this by a test-case? Otherwise, the change is looking good to me, thanks!

I'd like to, but I'm not sure if we can. I'll take a closer look today. Maybe just a targeted test of this function instead of needing multiple copies of mocked state.json files.

Patch:
diff --git a/internal/store/json.go b/internal/store/json.go
index 1a6cc86fe..8f5cbc499 100644
--- a/internal/store/json.go
+++ b/internal/store/json.go
@@ -186,7 +186,7 @@ func newChangesFromV0(changesStruct changesV0) map[string]map[string]blueprint.C
 func newCommitsFromV0(commitsMapStruct commitsV0, changesMapStruct changesV0) map[string][]string {
 	commitsMap := make(map[string][]string)
 	for name, commitsStruct := range commitsMapStruct {
-		commits := make([]string, 0, len(commitsStruct))
+		commits := make([]string, len(commitsStruct))
 		copy(commits, commitsStruct)
 		commitsMap[name] = commits
 	}
diff --git a/osbuild b/osbuild
index c1a98af56..f92a00b89 160000
--- a/osbuild
+++ b/osbuild
@@ -1,1 +1,1 @@
-Subproject commit c1a98af56acf376f2b83f6b4f8ae7c984b03f5be
+Subproject commit f92a00b891bab730ca1e4fd0c20d13d929999624
diff --git a/schutzbot/Jenkinsfile b/schutzbot/Jenkinsfile
index 3b43a3b92..1b88bb7d9 100644
--- a/schutzbot/Jenkinsfile
+++ b/schutzbot/Jenkinsfile
@@ -19,6 +19,11 @@ pipeline {
 
         stage("Prepare 🤔") {
             agent { label "schutzbot" }
+            options {
+                // Don't checkout the git repository here. It just clogs
+                // up the Jenkins disk space and does nothing for us.
+                skipDefaultCheckout()
+            }
             steps {
                 sh (
                     label: "Get environment variables",


Test patch:
diff --git a/internal/store/json_test.go b/internal/store/json_test.go
index 1c0dd85ce..92c6490e0 100644
--- a/internal/store/json_test.go
+++ b/internal/store/json_test.go
@@ -232,3 +232,88 @@ func Test_upgrade(t *testing.T) {
 		assert.Equal(1, len(store.workspace))
 	}
 }
+
+func Test_newCommitsFromV0(t *testing.T) {
+	exampleChanges := changesV0{
+		"test-blueprint-changes-v0": {
+			"4774980638f4162d9909a613c3ccd938e60bb3a9": {
+				Commit:    "4774980638f4162d9909a613c3ccd938e60bb3a9",
+				Message:   "Recipe test-blueprint-changes-v0, version 0.1.2 saved.",
+				Revision:  nil,
+				Timestamp: "2020-07-29T09:52:07Z",
+			},
+			"72fdb76b9994bd72770e283bf3a5206756daf497": {
+				Commit:    "72fdb76b9994bd72770e283bf3a5206756daf497",
+				Message:   "Recipe test-blueprint-changes-v0, version 0.1.1 saved.",
+				Revision:  nil,
+				Timestamp: "2020-07-09T13:33:06Z",
+			},
+			"79e2043a83637ffdd4db078c6da23deaae09c84b": {
+				Commit:    "79e2043a83637ffdd4db078c6da23deaae09c84b",
+				Message:   "Recipe test-blueprint-changes-v0, version 0.0.1 saved.",
+				Revision:  nil,
+				Timestamp: "2020-07-07T02:57:00Z",
+			},
+		},
+	}
+	type args struct {
+		changes changesV0
+		commits commitsV0
+	}
+	tests := []struct {
+		name string
+		args args
+		want map[string][]string
+	}{
+		{
+			name: "empty",
+			args: args{
+				changes: make(changesV0),
+				commits: make(commitsV0),
+			},
+			want: make(map[string][]string),
+		},
+		{
+			name: "Changes with no commits",
+			args: args{
+				changes: exampleChanges,
+				commits: make(commitsV0),
+			},
+			want: map[string][]string{
+				"test-blueprint-changes-v0": {
+					// Oldest sorted first
+					"79e2043a83637ffdd4db078c6da23deaae09c84b",
+					"72fdb76b9994bd72770e283bf3a5206756daf497",
+					"4774980638f4162d9909a613c3ccd938e60bb3a9",
+				},
+			},
+		},
+		{
+			name: "Changes with commits",
+			args: args{
+				changes: exampleChanges,
+				commits: commitsV0{
+					"test-blueprint-changes-v0": {
+						"79e2043a83637ffdd4db078c6da23deaae09c84b",
+						"72fdb76b9994bd72770e283bf3a5206756daf497",
+						"4774980638f4162d9909a613c3ccd938e60bb3a9",
+					},
+				},
+			},
+			want: map[string][]string{
+				"test-blueprint-changes-v0": {
+					// Oldest sorted first
+					"79e2043a83637ffdd4db078c6da23deaae09c84b",
+					"72fdb76b9994bd72770e283bf3a5206756daf497",
+					"4774980638f4162d9909a613c3ccd938e60bb3a9",
+				},
+			},
+		}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := newCommitsFromV0(tt.args.commits, tt.args.changes); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("newCommitsFromV0() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}