disk: cover all entities in PartitionTable.features()

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

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
 	}

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))
 	}
 }

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)