Merge pull request #38908 from spxtr/wow

Automatic merge from submit-queue

Remove two zany unit tests.

These two tests aren't unit tests in the usual sense. We can consider switching them to run as verify checks, but I'm not convinced that they're even necessary.

They essentially work by searching their code for public functions with signatures that look like `FitPredicate`, then they shell out to grep to see that they're used somewhere in the source tree. This will never work in bazel.

pkg/util/codeinspector/BUILD

@@ -1,18 +0,0 @@
-package(default_visibility = ["//visibility:public"])
-
-licenses(["notice"])
-
-load(
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-)
-
-go_library(
-    name = "go_default_library",
-    srcs = ["codeinspector.go"],
-    tags = ["automanaged"],
-    deps = [
-        "//vendor:k8s.io/gengo/parser",
-        "//vendor:k8s.io/gengo/types",
-    ],
-)

pkg/util/codeinspector/codeinspector.go

@@ -1,96 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package codeinspector
-
-import (
-	"fmt"
-	"go/ast"
-	"go/parser"
-	"go/token"
-	"io/ioutil"
-	"strings"
-	"unicode"
-
-	go2idlparser "k8s.io/gengo/parser"
-	"k8s.io/gengo/types"
-)
-
-// GetPublicFunctions lists all public functions (not methods) from a golang source file.
-func GetPublicFunctions(pkg, filePath string) ([]*types.Type, error) {
-	builder := go2idlparser.New()
-	data, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		return nil, err
-	}
-	if err := builder.AddFile(pkg, filePath, data); err != nil {
-		return nil, err
-	}
-	universe, err := builder.FindTypes()
-	if err != nil {
-		return nil, err
-	}
-
-	var functions []*types.Type
-
-	// Create the AST by parsing src.
-	fset := token.NewFileSet() // positions are relative to fset
-	f, err := parser.ParseFile(fset, filePath, nil, 0)
-	if err != nil {
-		return nil, fmt.Errorf("failed parse file to list functions: %v", err)
-	}
-
-	// Inspect the AST and print all identifiers and literals.
-	ast.Inspect(f, func(n ast.Node) bool {
-		var s string
-		switch x := n.(type) {
-		case *ast.FuncDecl:
-			s = x.Name.Name
-			// It's a function (not method), and is public, record it.
-			if x.Recv == nil && isPublic(s) {
-				functions = append(functions, universe[pkg].Function(x.Name.Name))
-			}
-		}
-		return true
-	})
-
-	return functions, nil
-}
-
-// isPublic checks if a given string is a public function name.
-func isPublic(myString string) bool {
-	a := []rune(myString)
-	a[0] = unicode.ToUpper(a[0])
-	return myString == string(a)
-}
-
-// GetSourceCodeFiles lists golang source code files from directory, excluding sub-directory and tests files.
-func GetSourceCodeFiles(dir string) ([]string, error) {
-	files, err := ioutil.ReadDir(dir)
-	if err != nil {
-		return nil, err
-	}
-
-	var filenames []string
-
-	for _, file := range files {
-		if strings.HasSuffix(file.Name(), ".go") && !strings.HasSuffix(file.Name(), "_test.go") {
-			filenames = append(filenames, file.Name())
-		}
-	}
-
-	return filenames, nil
-}

plugin/pkg/scheduler/algorithm/predicates/BUILD

@@ -39,19 +39,13 @@ go_test(
         "utils_test.go",
     ],
     library = "go_default_library",
-    tags = [
+    tags = ["automanaged"],
-        "automanaged",
-        "skip",
-    ],
     deps = [
         "//pkg/api/resource:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/labels:go_default_library",
-        "//pkg/util/codeinspector:go_default_library",
         "//plugin/pkg/scheduler/algorithm:go_default_library",
         "//plugin/pkg/scheduler/algorithm/priorities/util:go_default_library",
         "//plugin/pkg/scheduler/schedulercache:go_default_library",
-        "//vendor:k8s.io/gengo/parser",
-        "//vendor:k8s.io/gengo/types",
     ],
 )

plugin/pkg/scheduler/algorithm/predicates/predicates_test.go

@@ -18,17 +18,12 @@ package predicates
 
 import (
 	"fmt"
-	"os/exec"
-	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
 
-	"k8s.io/gengo/parser"
-	"k8s.io/gengo/types"
 	"k8s.io/kubernetes/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/util/codeinspector"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
 	priorityutil "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities/util"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
@@ -1849,84 +1844,6 @@ func TestEBSVolumeCountConflicts(t *testing.T) {
 	}
 }
 
-func getPredicateSignature() (*types.Signature, error) {
-	filePath := "./../types.go"
-	pkgName := filepath.Dir(filePath)
-	builder := parser.New()
-	if err := builder.AddDir(pkgName); err != nil {
-		return nil, err
-	}
-	universe, err := builder.FindTypes()
-	if err != nil {
-		return nil, err
-	}
-	result, ok := universe[pkgName].Types["FitPredicate"]
-	if !ok {
-		return nil, fmt.Errorf("FitPredicate type not defined")
-	}
-	return result.Signature, nil
-}
-
-func TestPredicatesRegistered(t *testing.T) {
-	var functions []*types.Type
-
-	// Files and directories which predicates may be referenced
-	targetFiles := []string{
-		"./../../algorithmprovider/defaults/defaults.go", // Default algorithm
-		"./../../factory/plugins.go",                     // Registered in init()
-		"./../../../../../pkg/",                          // kubernetes/pkg, often used by kubelet or controller
-	}
-
-	// List all golang source files under ./predicates/, excluding test files and sub-directories.
-	files, err := codeinspector.GetSourceCodeFiles(".")
-
-	if err != nil {
-		t.Errorf("unexpected error: %v when listing files in current directory", err)
-	}
-
-	// Get all public predicates in files.
-	for _, filePath := range files {
-		fileFunctions, err := codeinspector.GetPublicFunctions("k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/predicates", filePath)
-		if err == nil {
-			functions = append(functions, fileFunctions...)
-		} else {
-			t.Errorf("unexpected error %s when parsing %s", err, filePath)
-		}
-	}
-
-	predSignature, err := getPredicateSignature()
-	if err != nil {
-		t.Fatalf("Couldn't get predicates signature")
-	}
-
-	// Check if all public predicates are referenced in target files.
-	for _, function := range functions {
-		// Ignore functions that don't match FitPredicate signature.
-		signature := function.Underlying.Signature
-		if len(predSignature.Parameters) != len(signature.Parameters) {
-			continue
-		}
-		if len(predSignature.Results) != len(signature.Results) {
-			continue
-		}
-		// TODO: Check exact types of parameters and results.
-
-		args := []string{"-rl", function.Name.Name}
-		args = append(args, targetFiles...)
-
-		err := exec.Command("grep", args...).Run()
-		if err != nil {
-			switch err.Error() {
-			case "exit status 2":
-				t.Errorf("unexpected error when checking %s", function.Name)
-			case "exit status 1":
-				t.Errorf("predicate %s is implemented as public but seems not registered or used in any other place",
-					function.Name)
-			}
-		}
-	}
-}
-
 func newPodWithPort(hostPorts ...int) *v1.Pod {
 	networkPorts := []v1.ContainerPort{}
 	for _, port := range hostPorts {

plugin/pkg/scheduler/algorithm/priorities/BUILD

@@ -52,26 +52,19 @@ go_test(
         "node_affinity_test.go",
         "node_label_test.go",
         "node_prefer_avoid_pods_test.go",
-        "priorities_test.go",
         "selector_spreading_test.go",
         "taint_toleration_test.go",
     ],
     library = "go_default_library",
-    tags = [
+    tags = ["automanaged"],
-        "automanaged",
-        "skip",
-    ],
     deps = [
         "//pkg/api/resource:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/apis/meta/v1:go_default_library",
-        "//pkg/util/codeinspector:go_default_library",
         "//plugin/pkg/scheduler/algorithm:go_default_library",
         "//plugin/pkg/scheduler/algorithm/priorities/util:go_default_library",
         "//plugin/pkg/scheduler/api:go_default_library",
         "//plugin/pkg/scheduler/schedulercache:go_default_library",
-        "//vendor:k8s.io/gengo/parser",
-        "//vendor:k8s.io/gengo/types",
     ],
 )

plugin/pkg/scheduler/algorithm/priorities/priorities_test.go

@@ -1,117 +0,0 @@
-/*
-Copyright 2014 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package priorities
-
-import (
-	"fmt"
-	"os/exec"
-	"path/filepath"
-	"testing"
-
-	"k8s.io/gengo/parser"
-	"k8s.io/gengo/types"
-	"k8s.io/kubernetes/pkg/util/codeinspector"
-)
-
-func getPrioritySignatures() ([]*types.Signature, error) {
-	filePath := "./../types.go"
-	pkgName := filepath.Dir(filePath)
-	builder := parser.New()
-	if err := builder.AddDir(pkgName); err != nil {
-		return nil, err
-	}
-	universe, err := builder.FindTypes()
-	if err != nil {
-		return nil, err
-	}
-	signatures := []string{"PriorityFunction", "PriorityMapFunction", "PriorityReduceFunction"}
-	results := make([]*types.Signature, 0, len(signatures))
-	for _, signature := range signatures {
-		result, ok := universe[pkgName].Types[signature]
-		if !ok {
-			return nil, fmt.Errorf("%s type not defined", signature)
-		}
-		results = append(results, result.Signature)
-	}
-	return results, nil
-}
-
-func TestPrioritiesRegistered(t *testing.T) {
-	var functions []*types.Type
-
-	// Files and directories which priorities may be referenced
-	targetFiles := []string{
-		"./../../algorithmprovider/defaults/defaults.go", // Default algorithm
-		"./../../factory/plugins.go",                     // Registered in init()
-	}
-
-	// List all golang source files under ./priorities/, excluding test files and sub-directories.
-	files, err := codeinspector.GetSourceCodeFiles(".")
-
-	if err != nil {
-		t.Errorf("unexpected error: %v when listing files in current directory", err)
-	}
-
-	// Get all public priorities in files.
-	for _, filePath := range files {
-		fileFunctions, err := codeinspector.GetPublicFunctions("k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities", filePath)
-		if err == nil {
-			functions = append(functions, fileFunctions...)
-		} else {
-			t.Errorf("unexpected error when parsing %s: %v", filePath, err)
-		}
-	}
-
-	prioritySignatures, err := getPrioritySignatures()
-	if err != nil {
-		t.Fatalf("Couldn't get priorities signatures")
-	}
-
-	// Check if all public priorities are referenced in target files.
-	for _, function := range functions {
-		// Ignore functions that don't match priorities signatures.
-		signature := function.Underlying.Signature
-		match := false
-		for _, prioritySignature := range prioritySignatures {
-			if len(prioritySignature.Parameters) != len(signature.Parameters) {
-				continue
-			}
-			if len(prioritySignature.Results) != len(signature.Results) {
-				continue
-			}
-			// TODO: Check exact types of parameters and results.
-			match = true
-		}
-		if !match {
-			continue
-		}
-
-		args := []string{"-rl", function.Name.Name}
-		args = append(args, targetFiles...)
-
-		err := exec.Command("grep", args...).Run()
-		if err != nil {
-			switch err.Error() {
-			case "exit status 2":
-				t.Errorf("unexpected error when checking %s", function.Name)
-			case "exit status 1":
-				t.Errorf("priority %s is implemented as public but seems not registered or used in any other place",
-					function.Name)
-			}
-		}
-	}
-}