diff --git a/pkg/disk/disk_test.go b/pkg/disk/disk_test.go index 372b073c0c..15b40ece46 100644 --- a/pkg/disk/disk_test.go +++ b/pkg/disk/disk_test.go @@ -987,22 +987,23 @@ func TestForEachFSTabEntity(t *testing.T) { } for name := range testdisk.TestPartitionTables { - t.Run(name, func(t *testing.T) { - + // use a different name for the internal testing argument so we can + // refer to the global test by t.Name() in the error message + t.Run(name, func(ts *testing.T) { var targets []string targetCollectorCB := func(ent disk.FSTabEntity, _ []disk.Entity) error { targets = append(targets, ent.GetFSFile()) return nil } - require := require.New(t) + require := require.New(ts) pt := testdisk.TestPartitionTables[name] // print an informative failure message if a new test partition // table is added and this test is not updated (instead of failing // at the final Equal() check) exp, ok := expectedEntityPaths[name] - require.True(ok, "expected options not defined for test partition table %q: please update the TestNewFSTabStageOptions test", name) + require.True(ok, "expected test result not defined for test partition table %q: please update the %s test", name, t.Name()) err := pt.ForEachFSTabEntity(targetCollectorCB) // the callback never returns an error, but let's check it anyway diff --git a/pkg/disk/partition_table.go b/pkg/disk/partition_table.go index 04c64ecda7..1fc4199afb 100644 --- a/pkg/disk/partition_table.go +++ b/pkg/disk/partition_table.go @@ -841,16 +841,20 @@ type partitionTableFeatures struct { Swap bool } -// features examines all of the PartitionTable entities -// and returns a struct with flags set for each feature used +// features examines all of the PartitionTable entities and returns a struct +// with flags set for each feature used. The meaning of "feature" here is quite +// broad. Most disk Entity types are represented by a feature and the existence +// of at least one type in the partition table means the feature is +// represented. For Filesystem entities, there is a separate feature for each +// filesystem type func (pt *PartitionTable) features() partitionTableFeatures { var ptFeatures partitionTableFeatures introspectPT := func(e Entity, path []Entity) error { switch ent := e.(type) { - case *LVMLogicalVolume: + case *LVMVolumeGroup, *LVMLogicalVolume: ptFeatures.LVM = true - case *Btrfs: + case *Btrfs, *BtrfsSubvolume: ptFeatures.Btrfs = true case *Filesystem: switch ent.GetFSType() { @@ -867,6 +871,10 @@ func (pt *PartitionTable) features() partitionTableFeatures { ptFeatures.Swap = true case *LUKSContainer: ptFeatures.LUKS = true + case *PartitionTable, *Partition: + // nothing to do + default: + panic(fmt.Errorf("unknown entity type %T", e)) } return nil } diff --git a/pkg/disk/partition_table_test.go b/pkg/disk/partition_table_test.go index 2924a5f00b..7377bf5cac 100644 --- a/pkg/disk/partition_table_test.go +++ b/pkg/disk/partition_table_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/osbuild/images/internal/testdisk" "github.com/osbuild/images/pkg/blueprint" @@ -2284,21 +2285,24 @@ func TestNewCustomPartitionTableErrors(t *testing.T) { } func TestPartitionTableFeatures(t *testing.T) { - type testCase struct { - partitionType string - expectedFeatures disk.PartitionTableFeatures - } - testCases := []testCase{ - {"plain", disk.PartitionTableFeatures{XFS: true, FAT: true}}, - {"plain-swap", disk.PartitionTableFeatures{XFS: true, FAT: true, Swap: true}}, - {"luks", disk.PartitionTableFeatures{XFS: true, FAT: true, LUKS: true}}, - {"luks+lvm", disk.PartitionTableFeatures{XFS: true, FAT: true, LUKS: true, LVM: true}}, - {"btrfs", disk.PartitionTableFeatures{XFS: true, FAT: true, Btrfs: true}}, - } + require := require.New(t) - for _, tc := range testCases { - pt := testdisk.TestPartitionTables[tc.partitionType] - assert.Equal(t, tc.expectedFeatures, disk.GetPartitionTableFeatures(pt)) + testCases := map[string]disk.PartitionTableFeatures{ + "plain": {XFS: true, FAT: true}, + "plain-noboot": {XFS: true, FAT: true}, + "plain-swap": {XFS: true, FAT: true, Swap: true}, + "luks": {XFS: true, FAT: true, LUKS: true}, + "luks+lvm": {XFS: true, FAT: true, LUKS: true, LVM: true}, + "btrfs": {XFS: true, FAT: true, Btrfs: true}, + } + for name := range testdisk.TestPartitionTables { + // print an informative failure message if a new test partition + // table is added and this test is not updated (instead of failing + // at the final Equal() check) + exp, ok := testCases[name] + require.True(ok, "expected test result not defined for test partition table %q: please update the %s test", name, t.Name()) + pt := testdisk.TestPartitionTables[name] + require.Equal(exp, disk.GetPartitionTableFeatures(pt)) } } diff --git a/pkg/osbuild/fstab_stage_test.go b/pkg/osbuild/fstab_stage_test.go index cd12f28246..6797980bf0 100644 --- a/pkg/osbuild/fstab_stage_test.go +++ b/pkg/osbuild/fstab_stage_test.go @@ -104,8 +104,10 @@ func TestNewFSTabStageOptions(t *testing.T) { } // Use the test partition tables from the disk package. for name := range testdisk.TestPartitionTables { - t.Run(name, func(t *testing.T) { - require := require.New(t) + // use a different name for the internal testing argument so we can + // refer to the global test by t.Name() in the error message + t.Run(name, func(ts *testing.T) { + require := require.New(ts) pt := testdisk.TestPartitionTables[name] // math/rand is good enough in this case @@ -118,7 +120,7 @@ func TestNewFSTabStageOptions(t *testing.T) { // table is added and this test is not updated (instead of failing // at the final Equal() check) exp, ok := expectedOptions[name] - require.True(ok, "expected options not defined for test partition table %q: please update the TestNewFSTabStageOptions test", name) + require.True(ok, "expected test result not defined for test partition table %q: please update the %s test", name, t.Name()) options, err := NewFSTabStageOptions(&pt) require.NoError(err)