updates based on feedback

copyFile is no longer attached to the build struct. getRPMFileNames now checks for the .rpm extension, to remove the previously unnecessary (and frankly ugly) logic. I also added more abstraction for readability. Finally, cleaned up the tests based on feedback and altered them to match changes.
suse-edge · Nov 1, 2023 · 5d8e2ca · 5d8e2ca
1 parent fa5e60b
commit 5d8e2ca
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 42 deletions.
diff --git a/.idea/.gitignore b/.idea/.gitignore
diff --git a/pkg/build/build.go b/pkg/build/build.go
@@ -180,7 +180,7 @@ func (b *Builder) generateBaseImageFilename() string {
 	return filename
 }
 
-func (b *Builder) copyFile(sourcePath string, destPath string) error {
+func copyFile(sourcePath string, destPath string) error {
 	sourceFile, err := os.Open(sourcePath)
 	if err != nil {
 		return fmt.Errorf("opening file from source path: %w", err)

diff --git a/pkg/build/rpm.go b/pkg/build/rpm.go
@@ -6,43 +6,43 @@ import (
 	"path/filepath"
 )
 
-func (b *Builder) getRPMFileNames() ([]string, error) {
+func (b *Builder) getRPMFileNames(rpmSourceDir string) ([]string, error) {
 	var rpmFileNames []string
-	rpmSourceDir := filepath.Join(b.buildConfig.ImageConfigDir, "rpms")
 
 	rpms, err := os.ReadDir(rpmSourceDir)
 	if err != nil {
 		return nil, fmt.Errorf("reading rpm source dir: %w", err)
 	}
 
 	for _, rpmFile := range rpms {
-		rpmFileNames = append(rpmFileNames, rpmFile.Name())
+		if filepath.Ext(rpmFile.Name()) == ".rpm" {
+			rpmFileNames = append(rpmFileNames, rpmFile.Name())
+		}
 	}
 
 	if len(rpmFileNames) == 0 {
 		return nil, fmt.Errorf("no rpms found")
-	} else if len(rpmFileNames) == 1 && rpmFileNames[0] == ".gitkeep" {
-		return nil, fmt.Errorf("no rpms found")
 	}
 
 	return rpmFileNames, nil
 }
 
 func (b *Builder) copyRPMs() error {
-	rpmFileNames, err := b.getRPMFileNames()
+	rpmSourceDir := filepath.Join(b.buildConfig.ImageConfigDir, "rpms")
+	rpmDestDir := b.combustionDir
+
+	rpmFileNames, err := b.getRPMFileNames(rpmSourceDir)
 	if err != nil {
 		return fmt.Errorf("getting rpm file names: %w", err)
 	}
 
-	rpmSourceDir := filepath.Join(b.buildConfig.ImageConfigDir, "rpms")
-
 	for _, rpm := range rpmFileNames {
 		sourcePath := filepath.Join(rpmSourceDir, rpm)
-		destPath := filepath.Join(b.combustionDir, rpm)
+		destPath := filepath.Join(rpmDestDir, rpm)
 
-		err = b.copyFile(sourcePath, destPath)
+		err = copyFile(sourcePath, destPath)
 		if err != nil {
-			return fmt.Errorf("looping through rpms to copy: %w", err)
+			return fmt.Errorf("copying file %s: %w", sourcePath, err)
 		}
 	}
 

diff --git a/pkg/build/rpm_test.go b/pkg/build/rpm_test.go
@@ -20,17 +20,20 @@ func TestGetRPMFileNames(t *testing.T) {
 	require.NoError(t, err)
 	defer os.Remove(builder.eibBuildDir)
 
-	file1Path := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms", "rpm1.rpm")
-	file2Path := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms", "rpm2.rpm")
+	rpmSourceDir := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms")
 
+	file1Path := filepath.Join(rpmSourceDir, "rpm1.rpm")
+	defer os.Remove(file1Path)
 	file1, err := os.Create(file1Path)
 	require.NoError(t, err)
 
+	file2Path := filepath.Join(rpmSourceDir, "rpm2.rpm")
+	defer os.Remove(file2Path)
 	file2, err := os.Create(file2Path)
 	require.NoError(t, err)
 
 	// Test
-	rpmFileNames, err := builder.getRPMFileNames()
+	rpmFileNames, err := builder.getRPMFileNames(rpmSourceDir)
 
 	// Verify
 	require.NoError(t, err)
@@ -39,17 +42,8 @@ func TestGetRPMFileNames(t *testing.T) {
 	assert.Contains(t, rpmFileNames, "rpm2.rpm")
 
 	// Cleanup
-	err = file1.Close()
-	require.NoError(t, err)
-
-	err = file2.Close()
-	require.NoError(t, err)
-
-	err = os.Remove(file1Path)
-	require.NoError(t, err)
-
-	err = os.Remove(file2Path)
-	require.NoError(t, err)
+	assert.NoError(t, file1.Close())
+	assert.NoError(t, file2.Close())
 }
 
 func TestCopyRPMs(t *testing.T) {
@@ -62,12 +56,16 @@ func TestCopyRPMs(t *testing.T) {
 	require.NoError(t, err)
 	defer os.Remove(builder.eibBuildDir)
 
-	file1Path := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms", "rpm1.rpm")
-	file2Path := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms", "rpm2.rpm")
+	rpmSourceDir := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms")
+	rpmDestDir := filepath.Join(builder.combustionDir)
 
+	file1Path := filepath.Join(rpmSourceDir, "rpm1.rpm")
+	defer os.Remove(file1Path)
 	file1, err := os.Create(file1Path)
 	require.NoError(t, err)
 
+	file2Path := filepath.Join(rpmSourceDir, "rpm2.rpm")
+	defer os.Remove(file2Path)
 	file2, err := os.Create(file2Path)
 	require.NoError(t, err)
 
@@ -77,24 +75,15 @@ func TestCopyRPMs(t *testing.T) {
 	// Verify
 	require.NoError(t, err)
 
-	_, err = os.Stat(filepath.Join(builder.combustionDir, "rpm1.rpm"))
+	_, err = os.Stat(filepath.Join(rpmDestDir, "rpm1.rpm"))
 	require.NoError(t, err)
 
-	_, err = os.Stat(filepath.Join(builder.combustionDir, "rpm2.rpm"))
+	_, err = os.Stat(filepath.Join(rpmDestDir, "rpm2.rpm"))
 	require.NoError(t, err)
 
 	// Cleanup
-	err = file1.Close()
-	require.NoError(t, err)
-
-	err = file2.Close()
-	require.NoError(t, err)
-
-	err = os.Remove(file1Path)
-	require.NoError(t, err)
-
-	err = os.Remove(file2Path)
-	require.NoError(t, err)
+	assert.NoError(t, file1.Close())
+	assert.NoError(t, file2.Close())
 }
 
 func TestGetRPMFileNamesNoRPMs(t *testing.T) {
@@ -107,8 +96,10 @@ func TestGetRPMFileNamesNoRPMs(t *testing.T) {
 	require.NoError(t, err)
 	defer os.Remove(builder.eibBuildDir)
 
+	rpmSourceDir := filepath.Join(builder.buildConfig.ImageConfigDir, "rpms")
+
 	// Test
-	rpmFileNames, err := builder.getRPMFileNames()
+	rpmFileNames, err := builder.getRPMFileNames(rpmSourceDir)
 
 	// Verify
 	require.ErrorContains(t, err, "no rpms found")