Add unit test to function GetCPUVariantFromArch

Add unit test to function GetCPUVariantFromArch Fix import issue on non-linux platforms Fix some style issue Signed-off-by: Tony Fang <nenghui.fang@gmail.com>
kzys · Dec 20, 2022 · 6e55234 · 6e55234
1 parent 8d5c045
commit 6e55234
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 13 deletions.
diff --git a/platforms/cpuinfo.go b/platforms/cpuinfo.go
@@ -35,7 +35,7 @@ func cpuVariant() string {
 			var err error
 			cpuVariantValue, err = getCPUVariant()
 			if err != nil {
-				log.L.Errorf("Error getCPUVariant for OS %s : %v", runtime.GOOS, err)
+				log.L.Errorf("Error getCPUVariant for OS %s: %v", runtime.GOOS, err)
 			}
 		}
 	})

diff --git a/platforms/cpuinfo_linux.go b/platforms/cpuinfo_linux.go
@@ -70,7 +70,7 @@ func getCPUInfo(pattern string) (info string, err error) {
 		return "", err
 	}
 
-	return "", fmt.Errorf("getCPUInfo for pattern: %s: %w", pattern, errdefs.ErrNotFound)
+	return "", fmt.Errorf("getCPUInfo for pattern %s: %w", pattern, errdefs.ErrNotFound)
 }
 
 // getCPUVariantFromArch get CPU variant from arch through a system call
@@ -82,7 +82,7 @@ func getCPUVariantFromArch(arch string) (string, error) {
 
 	if arch == "aarch64" {
 		variant = "8"
-	} else if len(arch) >= 5 {
+	} else if arch[0:4] == "armv" && len(arch) >= 5 {
 		//Valid arch format is in form of armvXx
 		switch arch[3:5] {
 		case "v8":
@@ -101,7 +101,7 @@ func getCPUVariantFromArch(arch string) (string, error) {
 			variant = "unknown"
 		}
 	} else {
-		return "", fmt.Errorf("getCPUVariantFromArch invalid arch : %s, %v", arch, errdefs.ErrInvalidArgument)
+		return "", fmt.Errorf("getCPUVariantFromArch invalid arch: %s, %w", arch, errdefs.ErrInvalidArgument)
 	}
 	return variant, nil
 }
@@ -119,15 +119,15 @@ func getCPUVariant() (string, error) {
 			//Let's try getting CPU variant from machine architecture
 			arch, err := getMachineArch()
 			if err != nil {
-				return "", fmt.Errorf("failure getting machine architecture : %v", err)
+				return "", fmt.Errorf("failure getting machine architecture: %v", err)
 			}
 
 			variant, err = getCPUVariantFromArch(arch)
 			if err != nil {
-				return "", fmt.Errorf("failure getting CPU variant from machine architecture : %v", err)
+				return "", fmt.Errorf("failure getting CPU variant from machine architecture: %v", err)
 			}
 		} else {
-			return "", fmt.Errorf("failure getting CPU variant : %v", err)
+			return "", fmt.Errorf("failure getting CPU variant: %v", err)
 		}
 	}
 

diff --git a/platforms/cpuinfo_linux_test.go b/platforms/cpuinfo_linux_test.go
@@ -17,8 +17,11 @@
 package platforms
 
 import (
+	"errors"
 	"runtime"
 	"testing"
+
+	"github.com/containerd/containerd/errdefs"
 )
 
 func TestCPUVariant(t *testing.T) {
@@ -30,24 +33,109 @@ func TestCPUVariant(t *testing.T) {
 
 	p, err := getCPUVariant()
 	if err != nil {
-		t.Fatalf("Error getting CPU variant: %v\n", err)
+		t.Fatalf("Error getting CPU variant: %v", err)
 		return
 	}
 
 	for _, variant := range variants {
 		if p == variant {
-			t.Logf("got valid variant as expected: %#v = %#v\n", p, variant)
+			t.Logf("got valid variant as expected: %#v = %#v", p, variant)
 			return
 		}
 	}
 
-	t.Fatalf("could not get valid variant as expected: %v\n", variants)
+	t.Fatalf("could not get valid variant as expected: %v", variants)
 }
 
 func TestGetCPUVariantFromArch(t *testing.T) {
 
-	if !isArmArch(runtime.GOARCH) {
-		t.Skip("only relevant on linux/arm")
-	}
+	for _, testcase := range []struct {
+		name        string
+		input       string
+		output      string
+		expectedErr error
+	}{
+		{
+			name:        "Test aarch64",
+			input:       "aarch64",
+			output:      "8",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test Armv8 with capital",
+			input:       "Armv8",
+			output:      "8",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test armv7",
+			input:       "armv7",
+			output:      "7",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test armv6",
+			input:       "armv6",
+			output:      "6",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test armv5",
+			input:       "armv5",
+			output:      "5",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test armv4",
+			input:       "armv4",
+			output:      "4",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test armv3",
+			input:       "armv3",
+			output:      "3",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test unknown input",
+			input:       "armv9",
+			output:      "unknown",
+			expectedErr: nil,
+		},
+		{
+			name:        "Test invalid input which doesn't start with armv",
+			input:       "armxxxx",
+			output:      "",
+			expectedErr: errdefs.ErrInvalidArgument,
+		},
+		{
+			name:        "Test invalid input whose length is less than 5",
+			input:       "armv",
+			output:      "",
+			expectedErr: errdefs.ErrInvalidArgument,
+		},
+	} {
+		t.Run(testcase.name, func(t *testing.T) {
+			t.Logf("input: %v", testcase.input)
 
+			variant, err := getCPUVariantFromArch(testcase.input)
+
+			if err == nil {
+				if testcase.expectedErr != nil {
+					t.Fatalf("Expect to get error: %v, however no error got", testcase.expectedErr)
+				} else {
+					if variant != testcase.output {
+						t.Fatalf("Expect to get variant: %v, however %v returned", testcase.output, variant)
+					}
+				}
+
+			} else {
+				if !errors.Is(err, testcase.expectedErr) {
+					t.Fatalf("Expect to get error: %v, however error %v returned", testcase.expectedErr, err)
+				}
+			}
+		})
+
+	}
 }
diff --git a/platforms/cpuinfo_other.go b/platforms/cpuinfo_other.go
@@ -20,7 +20,10 @@
 package platforms
 
 import (
+	"fmt"
 	"runtime"
+
+	"github.com/containerd/containerd/errdefs"
 )
 
 func getCPUVariant() (string, error) {