Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slicer/test: Support for custom packages #80

Closed
wants to merge 2 commits into from
Closed

slicer/test: Support for custom packages #80

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d5d78c
slicer/test: Support for custom packages
woky Jun 17, 2023
e453348
fixup! slicer/test: Support for custom packages
woky Oct 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 104 additions & 8 deletions internal/slicer/slicer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,20 @@ import (
"github.com/canonical/chisel/internal/testutil"
)

var (
Reg = testutil.Reg
Dir = testutil.Dir
Lnk = testutil.Lnk
)

type testPackage struct {
content []byte
}

type slicerTest struct {
summary string
arch string
pkgs map[string]map[string]testPackage
woky marked this conversation as resolved.
Show resolved Hide resolved
release map[string]string
slices []setup.SliceKey
hackopt func(c *C, opts *slicer.RunOptions)
Expand Down Expand Up @@ -515,6 +526,77 @@ var slicerTests = []slicerTest{{
"/usr/bin/": "dir 0755",
"/usr/bin/hello": "file 0775 eaf29575",
},
}, {
summary: "Custom archives with custom packages",
pkgs: map[string]map[string]testPackage{
"leptons": {
"electron": testPackage{
Copy link
Contributor

@niemeyer niemeyer Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither in the type definition nor here do we have a hint of what these strings actually mean. This also seems slightly misleading because the testPackage is actually a slice here, isn't it? No, it's a deb. So it's archive and packages.

We have @rebornplusplus above hinting as this issue too. I'm not sure his suggestion is the best idea either, but it would be nice to make things slightly cleaner. Any suggestions?

Copy link
Contributor Author

@woky woky Oct 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niemeyer I've applied @rebornplusplus's suggestion and in the end, I like the result. The definition now looks like:

	summary: "Custom archives with custom packages",
	archives: map[string]testArchiveData{
		"leptons": testArchiveData{
			packages: []testPackageData{{
				name: "electron",
				content: testutil.MustMakeDeb([]testutil.TarEntry{
					Dir(0755, "./"),
					Dir(0755, "./mass/"),

I've made archive packages a list instead of a map because one archive can in theory contain packages of the same name with different versions and/or architectures. It also reduces one level of indentation.

Note that there's also testArchive structure which implements Archive interface. That's why I suffixed the new structures with Data, though I admit it's kind of a fallback naming...

The changes are in a new fixup commit. Does it look better now?

content: testutil.MustMakeDeb([]testutil.TarEntry{
Dir(0755, "./"),
Dir(0755, "./mass/"),
Reg(0644, "./mass/electron", "9.1093837015E−31 kg\n"),
Dir(0755, "./usr/"),
Dir(0755, "./usr/share/"),
Dir(0755, "./usr/share/doc/"),
Dir(0755, "./usr/share/doc/electron/"),
Reg(0644, "./usr/share/doc/electron/copyright", ""),
}),
},
},
"hadrons": {
"proton": testPackage{
content: testutil.MustMakeDeb([]testutil.TarEntry{
Dir(0755, "./"),
Dir(0755, "./mass/"),
Reg(0644, "./mass/proton", "1.67262192369E−27 kg\n"),
}),
},
},
},
release: map[string]string{
"chisel.yaml": `
format: chisel-v1
archives:
leptons:
version: 1
suites: [main]
components: [main, universe]
default: true
hadrons:
version: 1
suites: [main]
components: [main]
`,
"slices/mydir/electron.yaml": `
package: electron
slices:
mass:
contents:
/mass/electron:
`,
"slices/mydir/proton.yaml": `
package: proton
archive: hadrons
slices:
mass:
contents:
/mass/proton:
`,
},
slices: []setup.SliceKey{
{"electron", "mass"},
{"proton", "mass"},
},
result: map[string]string{
"/mass/": "dir 0755",
"/mass/electron": "file 0644 a1258e30",
"/mass/proton": "file 0644 a2390d10",
"/usr/": "dir 0755",
"/usr/share/": "dir 0755",
"/usr/share/doc/": "dir 0755",
"/usr/share/doc/electron/": "dir 0755",
"/usr/share/doc/electron/copyright": "file 0644 empty",
},
}}

const defaultChiselYaml = `
Expand All @@ -527,7 +609,7 @@ const defaultChiselYaml = `

type testArchive struct {
options archive.Options
pkgs map[string][]byte
pkgs map[string]testPackage
}

func (a *testArchive) Options() *archive.Options {
Expand All @@ -536,7 +618,7 @@ func (a *testArchive) Options() *archive.Options {

func (a *testArchive) Fetch(pkg string) (io.ReadCloser, error) {
if data, ok := a.pkgs[pkg]; ok {
return io.NopCloser(bytes.NewBuffer(data)), nil
return io.NopCloser(bytes.NewBuffer(data.content)), nil
}
return nil, fmt.Errorf("attempted to open %q package", pkg)
}
Expand Down Expand Up @@ -569,16 +651,23 @@ func (s *S) TestRun(c *C) {
selection, err := setup.Select(release, test.slices)
c.Assert(err, IsNil)

pkgs := map[string][]byte{
"base-files": testutil.PackageData["base-files"],
pkgs := map[string]testPackage{
"base-files": testPackage{content: testutil.PackageData["base-files"]},
}
for name, entries := range packageEntries {
deb, err := testutil.MakeDeb(entries)
c.Assert(err, IsNil)
pkgs[name] = deb
pkgs[name] = testPackage{content: deb}
}
archives := map[string]archive.Archive{}
for name, setupArchive := range release.Archives {
var archivePkgs map[string]testPackage
if test.pkgs != nil {
archivePkgs = test.pkgs[name]
}
if archivePkgs == nil {
archivePkgs = pkgs
}
archive := &testArchive{
options: archive.Options{
Label: setupArchive.Name,
Expand All @@ -587,7 +676,7 @@ func (s *S) TestRun(c *C) {
Components: setupArchive.Components,
Arch: test.arch,
},
pkgs: pkgs,
pkgs: archivePkgs,
}
archives[name] = archive
}
Expand All @@ -611,8 +700,15 @@ func (s *S) TestRun(c *C) {

if test.result != nil {
result := make(map[string]string, len(copyrightEntries)+len(test.result))
for k, v := range copyrightEntries {
result[k] = v
if test.pkgs == nil {
// This was added in order to not specify copyright entries for each
// existing test. These tests use only the base-files embedded
// package. Custom packages may not include copyright entries
// though. So if a test defines any custom packages, it must include
// copyright entries explicitly in the results.
for k, v := range copyrightEntries {
result[k] = v
}
}
for k, v := range test.result {
result[k] = v
Expand Down