fix: address review comments

canonical · Sep 24, 2024 · 3d792c1 · 3d792c1
1 parent 35cee63
commit 3d792c1
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 47 deletions.
diff --git a/internal/setup/setup.go b/internal/setup/setup.go
@@ -817,6 +817,7 @@ func Select(release *Release, slices []SliceKey) (*Selection, error) {
 }
 
 // pathInfoToYAML converts a PathInfo object to a yamlPath object.
+// The returned object is going to have pointers to the given PathInfo object.
 func pathInfoToYAML(pi *PathInfo) (*yamlPath, error) {
 	path := &yamlPath{
 		Mode:    yamlMode(pi.Mode),
@@ -830,8 +831,7 @@ func pathInfoToYAML(pi *PathInfo) (*yamlPath, error) {
 	case CopyPath:
 		path.Copy = pi.Info
 	case TextPath:
-		text := pi.Info
-		path.Text = &text
+		path.Text = &pi.Info
 	case SymlinkPath:
 		path.Symlink = pi.Info
 	case GlobPath:
@@ -853,6 +853,8 @@ func sliceToYAML(s *Slice) (*yamlSlice, error) {
 		slice.Essential = append(slice.Essential, key.String())
 	}
 	for path, info := range s.Contents {
+		// TODO remove the following line after upgrading to Go 1.22 or higher.
+		info := info
 		yamlPath, err := pathInfoToYAML(&info)
 		if err != nil {
 			return nil, err

diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go
@@ -1687,55 +1687,121 @@ func (s *S) TestPackageMarshalYAML(c *C) {
 }
 
 func (s *S) TestPackageYAMLFormat(c *C) {
-	var input = map[string]string{
-		"chisel.yaml": string(defaultChiselYaml),
-		"slices/mypkg1.yaml": `
-			package: mypkg1
-			archive: ubuntu
-			slices:
-				myslice1:
-					contents:
-						/dir/file: {}
-		`,
-		"slices/mypkg3.yaml": `
-			package: mypkg3
-			archive: ubuntu
-			slices:
-				myslice:
-					essential:
-						- mypkg1_myslice1
-					contents:
-						/dir/arch-specific*: {arch: [amd64, arm64, i386]}
-						/dir/copy: {copy: /dir/file}
-						/dir/glob*: {}
-						/dir/mutable: {text: TODO, mutable: true, arch: riscv64}
-						/dir/other-file: {}
-						/dir/sub-dir/: {make: true, mode: 0644}
-						/dir/symlink: {symlink: /dir/file}
-						/dir/until: {until: mutate}
-					mutate: |
-						# Test multi-line string.
-						content.write("/dir/mutable", foo)
-		`,
-	}
+	var tests = []struct {
+		summary  string
+		input    map[string]string
+		expected map[string]string
+	}{{
+		summary: "Basic slice",
+		input: map[string]string{
+			"slices/mypkg.yaml": `
+				package: mypkg
+				archive: ubuntu
+				slices:
+					myslice:
+						contents:
+							/dir/file: {}
+			`,
+		},
+	}, {
+		summary: "All types of paths",
+		input: map[string]string{
+			"slices/mypkg.yaml": `
+				package: mypkg
+				archive: ubuntu
+				slices:
+					myslice:
+						contents:
+							/dir/arch-specific*: {arch: [amd64, arm64, i386]}
+							/dir/copy: {copy: /dir/file}
+							/dir/empty-file: {text: ""}
+							/dir/glob*: {}
+							/dir/mutable: {text: TODO, mutable: true, arch: riscv64}
+							/dir/other-file: {}
+							/dir/sub-dir/: {make: true, mode: 0644}
+							/dir/symlink: {symlink: /dir/file}
+							/dir/until: {until: mutate}
+						mutate: |
+							# Test multi-line string.
+							content.write("/dir/mutable", foo)
+			`,
+		},
+	}, {
+		summary: "Global and per-slice essentials",
+		input: map[string]string{
+			"slices/mypkg.yaml": `
+				package: mypkg
+				archive: ubuntu
+				essential:
+					- mypkg_myslice3
+				slices:
+					myslice1:
+						essential:
+							- mypkg_myslice2
+						contents:
+							/dir/file1: {}
+					myslice2:
+						contents:
+							/dir/file2: {}
+					myslice3:
+						contents:
+							/dir/file3: {}
+			`,
+		},
+		expected: map[string]string{
+			"slices/mypkg.yaml": `
+				package: mypkg
+				archive: ubuntu
+				slices:
+					myslice1:
+						essential:
+							- mypkg_myslice3
+							- mypkg_myslice2
+						contents:
+							/dir/file1: {}
+					myslice2:
+						essential:
+							- mypkg_myslice3
+						contents:
+							/dir/file2: {}
+					myslice3:
+						contents:
+							/dir/file3: {}
+			`,
+		},
+	}}
 
-	dir := c.MkDir()
-	for fname, data := range input {
-		fpath := filepath.Join(dir, fname)
-		err := os.MkdirAll(filepath.Dir(fpath), 0755)
-		c.Assert(err, IsNil)
-		err = os.WriteFile(fpath, testutil.Reindent(string(data)), 0644)
-		c.Assert(err, IsNil)
-	}
+	for _, test := range tests {
+		c.Logf("Summary: %s", test.summary)
 
-	release, err := setup.ReadRelease(dir)
-	c.Assert(err, IsNil)
+		if _, ok := test.input["chisel.yaml"]; !ok {
+			test.input["chisel.yaml"] = string(defaultChiselYaml)
+		}
 
-	for _, pkg := range release.Packages {
-		data, err := yaml.Marshal(pkg)
+		dir := c.MkDir()
+		for path, data := range test.input {
+			fpath := filepath.Join(dir, path)
+			err := os.MkdirAll(filepath.Dir(fpath), 0755)
+			c.Assert(err, IsNil)
+			err = os.WriteFile(fpath, testutil.Reindent(data), 0644)
+			c.Assert(err, IsNil)
+		}
+
+		release, err := setup.ReadRelease(dir)
 		c.Assert(err, IsNil)
-		expected := string(testutil.Reindent(input[pkg.Path]))
-		c.Assert(strings.TrimSpace(string(data)), Equals, strings.TrimSpace(expected))
+
+		for _, pkg := range release.Packages {
+			data, err := yaml.Marshal(pkg)
+			c.Assert(err, IsNil)
+			var expected string
+			if test.expected == nil || test.expected[pkg.Path] == "" {
+				expected = test.input[pkg.Path]
+			} else {
+				expected = test.expected[pkg.Path]
+			}
+			expected = string(testutil.Reindent(expected))
+			c.Assert(strings.TrimSpace(string(data)), Equals, strings.TrimSpace(expected))
+		}
 	}
 }