chore: refactor pre-commit workflow to correctly detect new lint issues (#12302)

Signed-off-by: kaikaila <[email protected]>

Author: kaikaila

.github/workflows/pre-commit.yml

@@ -12,18 +12,22 @@ jobs:
   pre-commit:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v5
-    - uses: actions/setup-python@v3
-    - uses: actions/setup-go@v5
-      with:
-        go-version-file: go.mod
-    - name: golangci-lint
-      uses: golangci/golangci-lint-action@v8
-      with:
-        version: v2.3
-        args: --new-from-rev HEAD
+      - uses: actions/checkout@v5
+        with:
+          fetch-depth: 0
+      - name: Fetch base branch
+        run: git fetch origin ${{ github.base_ref || github.ref_name }}
+      - uses: actions/setup-python@v3
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v8
+        with:
+          version: v2.3
+          args: --new --whole-files --new-from-merge-base=origin/${{ github.base_ref || github.ref_name }}
     #- uses: pre-commit/[email protected]
-    #  # This is set to only run the golangci-lint pre-commit hooks 
+    #  # This is set to only run the golangci-lint pre-commit hooks
     #  # Remove in a later PR to run all hooks
     #  with:
     #    go-version: '>=1.24.2'

.pre-commit-config.yaml

@@ -18,9 +18,9 @@ repos:
   - repo: https://github.com/PyCQA/flake8
     rev: 7.1.1
     hooks:
-    -   id: flake8
+      - id: flake8
         args:
-        - --select=W605
+          - --select=W605
   # required formatting jobs (run these last)
 
   # add comment "noqa" to ignore an import that should not be removed
@@ -60,7 +60,7 @@ repos:
       - id: golangci-lint
         name: golangci-lint
         description: Fast linters runner for Go.
-        entry: golangci-lint run --new-from-rev HEAD --fix
+        entry: golangci-lint run --new-from-rev HEAD --fix --whole-files
         types: [go]
         language: golang
         require_serial: true

backend/test/compiler/argo_ginkgo_test.go

@@ -26,7 +26,7 @@ import (
 	workflowutils "github.com/kubeflow/pipelines/backend/test/compiler/utils"
 	. "github.com/kubeflow/pipelines/backend/test/constants"
 	"github.com/kubeflow/pipelines/backend/test/logger"
-	"github.com/kubeflow/pipelines/backend/test/test_utils"
+	"github.com/kubeflow/pipelines/backend/test/testutil"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -38,7 +38,7 @@ var _ = BeforeEach(func() {
 })
 
 var _ = Describe("Verify Spec Compilation to Workflow >", Label(POSITIVE, WORKFLOW_COMPILER), func() {
-	pipelineFilePaths := test_utils.GetListOfAllFilesInDir(filepath.Join(pipelineFilesRootDir, pipelineDirectory))
+	pipelineFilePaths := testutil.GetListOfAllFilesInDir(filepath.Join(pipelineFilesRootDir, pipelineDirectory))
 
 	testParams := []struct {
 		compilerOptions argocompiler.Options
@@ -68,7 +68,7 @@ var _ = Describe("Verify Spec Compilation to Workflow >", Label(POSITIVE, WORKFL
 				compiledWorkflowFileName := fileNameWithoutExtension + ".yaml"
 				compiledWorkflowFilePath := filepath.Join(argoYAMLDir, compiledWorkflowFileName)
 				It(fmt.Sprintf("When I compile %s pipeline spec, then the compiled yaml should be %s", pipelineSpecFileName, compiledWorkflowFileName), func() {
-					test_utils.CheckIfSkipping(pipelineSpecFileName)
+					testutil.CheckIfSkipping(pipelineSpecFileName)
 					pipelineSpecs, platformSpec := workflowutils.LoadPipelineSpecsFromIR(pipelineSpecFilePath, param.compilerOptions.CacheDisabled, nil)
 					compiledWorkflow := workflowutils.GetCompiledArgoWorkflow(pipelineSpecs, platformSpec, &param.compilerOptions)
 					if *createMissingGoldenFiles || *updateGoldenFiles {

backend/test/compiler/compiler_suite_test.go

@@ -24,16 +24,16 @@ import (
 	"testing"
 
 	"github.com/kubeflow/pipelines/backend/test/logger"
-	"github.com/kubeflow/pipelines/backend/test/test_utils"
+	"github.com/kubeflow/pipelines/backend/test/testutil"
 
 	. "github.com/onsi/ginkgo/v2"
 	"github.com/onsi/ginkgo/v2/types"
 	. "github.com/onsi/gomega"
 )
 
-var pipelineFilesRootDir = test_utils.GetPipelineFilesDir()
+var pipelineFilesRootDir = testutil.GetPipelineFilesDir()
 var pipelineDirectory = "valid"
-var argoYAMLDir = filepath.Join(test_utils.GetTestDataDir(), "compiled-workflows")
+var argoYAMLDir = filepath.Join(testutil.GetTestDataDir(), "compiled-workflows")
 var updateGoldenFiles = flag.Bool("updateCompiledFiles", false, "update golden/expected compiled workflow files")
 var createMissingGoldenFiles = flag.Bool("createGoldenFiles", true, "create missing golden/expected compiled workflow files")
 
@@ -60,7 +60,7 @@ var _ = ReportAfterEach(func(specReport types.SpecReport) {
 		Expect(err).NotTo(HaveOccurred(), "Failed to get current directory")
 		testName := GinkgoT().Name()
 		testNameSplit := strings.Split(testName, ">")
-		test_utils.WriteLogFile(specReport, testNameSplit[len(testNameSplit)-1], filepath.Join(currentDir, testLogsDirectory))
+		testutil.WriteLogFile(specReport, testNameSplit[len(testNameSplit)-1], filepath.Join(currentDir, testLogsDirectory))
 	} else {
 		log.Printf("Test passed")
 	}

backend/test/compiler/compiler_visitor_test.go

@@ -24,7 +24,7 @@ import (
 	"github.com/kubeflow/pipelines/backend/src/v2/compiler"
 	workflowutils "github.com/kubeflow/pipelines/backend/test/compiler/utils"
 	. "github.com/kubeflow/pipelines/backend/test/constants"
-	"github.com/kubeflow/pipelines/backend/test/test_utils"
+	"github.com/kubeflow/pipelines/backend/test/testutil"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -116,7 +116,7 @@ var _ = Describe("Verify iteration over the pipeline components >", Label(POSITI
 			},
 		}
 		for _, testParam := range testParams {
-			pipelineSpecFilePath := filepath.Join(test_utils.GetValidPipelineFilesDir(), testParam.pipelineSpecPath)
+			pipelineSpecFilePath := filepath.Join(testutil.GetValidPipelineFilesDir(), testParam.pipelineSpecPath)
 			It(fmt.Sprintf("Load the the pipeline IR yaml %s, and verify the visited component", testParam.pipelineSpecPath), func() {
 				pipelineJob, platformSpec := workflowutils.LoadPipelineSpecsFromIR(pipelineSpecFilePath, false, nil)
 

backend/test/compiler/utils/workflow_utils.go

@@ -28,8 +28,7 @@ import (
 	"github.com/kubeflow/pipelines/api/v2alpha1/go/pipelinespec"
 	"github.com/kubeflow/pipelines/backend/src/v2/compiler/argocompiler"
 	"github.com/kubeflow/pipelines/backend/test/logger"
-	"github.com/kubeflow/pipelines/backend/test/test_utils"
-	utils "github.com/kubeflow/pipelines/backend/test/test_utils"
+	"github.com/kubeflow/pipelines/backend/test/testutil"
 
 	"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
 	"github.com/onsi/ginkgo/v2"
@@ -39,7 +38,7 @@ import (
 
 // LoadPipelineSpecsFromIR - Unmarshall Pipeline Spec IR into a tuple of (pipelinespec.PipelineJob, pipelinespec.SinglePlatformSpec)
 func LoadPipelineSpecsFromIR(pipelineIRFilePath string, cacheDisabled bool, defaultWorkspace *v1.PersistentVolumeClaimSpec) (*pipelinespec.PipelineJob, *pipelinespec.SinglePlatformSpec) {
-	pipelineSpecsFromFile := utils.ParseFileToSpecs(pipelineIRFilePath, cacheDisabled, defaultWorkspace)
+	pipelineSpecsFromFile := testutil.ParseFileToSpecs(pipelineIRFilePath, cacheDisabled, defaultWorkspace)
 	platformSpec := pipelineSpecsFromFile.PlatformSpec()
 	var singlePlatformSpec *pipelinespec.SinglePlatformSpec = nil
 	if platformSpec != nil {
@@ -49,6 +48,7 @@ func LoadPipelineSpecsFromIR(pipelineIRFilePath string, cacheDisabled bool, defa
 	pipelineSpecBytes, marshallingError := protojson.Marshal(pipelineSpecsFromFile.PipelineSpec())
 	gomega.Expect(marshallingError).NotTo(gomega.HaveOccurred(), "Failed to marshall pipeline spec")
 	err := json.Unmarshal(pipelineSpecBytes, &pipelineSpecMap)
+	gomega.Expect(err).NotTo(gomega.HaveOccurred(), "Failed to unmarshal pipeline spec into map")
 	pipelineSpecMapNew := make(map[string]interface{})
 	pipelineSpecMapNew["pipelineSpec"] = pipelineSpecMap
 	pipelineSpecBytes, marshallingError = json.Marshal(pipelineSpecMapNew)
@@ -86,7 +86,7 @@ func CreateCompiledWorkflowFile(compiledWorflow *v1alpha1.Workflow, compiledWork
 	ginkgo.GinkgoHelper()
 	fileContents, err := yaml.Marshal(compiledWorflow)
 	gomega.Expect(err).NotTo(gomega.HaveOccurred())
-	return test_utils.CreateFile(compiledWorkflowFilePath, [][]byte{fileContents})
+	return testutil.CreateFile(compiledWorkflowFilePath, [][]byte{fileContents})
 }
 
 // ConfigureCacheSettings - Add/Remove cache_disabled args in the workflow

backend/test/end2end/e2e_suite_test.go

@@ -26,7 +26,7 @@ import (
 	apiserver "github.com/kubeflow/pipelines/backend/src/common/client/api_server/v2"
 	"github.com/kubeflow/pipelines/backend/test/config"
 	"github.com/kubeflow/pipelines/backend/test/logger"
-	"github.com/kubeflow/pipelines/backend/test/test_utils"
+	"github.com/kubeflow/pipelines/backend/test/testutil"
 	"github.com/kubeflow/pipelines/backend/test/v2"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -65,8 +65,8 @@ var _ = BeforeSuite(func() {
 	var newPipelineClient func() (*apiserver.PipelineClient, error)
 	var newRunClient func() (*apiserver.RunClient, error)
 	var newExperimentClient func() (*apiserver.ExperimentClient, error)
-	clientConfig := test_utils.GetClientConfig(*config.Namespace)
-	k8Client, err = test_utils.CreateK8sClient()
+	clientConfig := testutil.GetClientConfig(*config.Namespace)
+	k8Client, err = testutil.CreateK8sClient()
 	Expect(err).To(BeNil(), "Failed to initialize K8s client")
 
 	if *config.KubeflowMode {
@@ -86,7 +86,7 @@ var _ = BeforeSuite(func() {
 			userToken = *config.AuthToken
 		} else {
 			logger.Log("Creating API Clients for Multi User Mode")
-			userToken = test_utils.CreateUserToken(k8Client, *config.UserNamespace, *config.UserServiceAccountName)
+			userToken = testutil.CreateUserToken(k8Client, *config.UserNamespace, *config.UserServiceAccountName)
 		}
 		newPipelineClient = func() (*apiserver.PipelineClient, error) {
 			return apiserver.NewMultiUserPipelineClient(clientConfig, userToken, *config.DebugMode)
@@ -131,7 +131,7 @@ var _ = BeforeEach(func() {
 
 	// Create Experiment so that we can use it to associate pipeline runs with
 	experimentName := fmt.Sprintf("E2EExperiment-%s", strconv.FormatInt(time.Now().UnixNano(), 10))
-	experiment := test_utils.CreateExperiment(experimentClient, experimentName, test_utils.GetNamespace())
+	experiment := testutil.CreateExperiment(experimentClient, experimentName, testutil.GetNamespace())
 	experimentID = &experiment.ExperimentID
 })
 
@@ -141,7 +141,7 @@ var _ = ReportAfterEach(func(specReport types.SpecReport) {
 		AddReportEntry("Test Log", specReport.CapturedGinkgoWriterOutput)
 		currentDir, err := os.Getwd()
 		Expect(err).NotTo(HaveOccurred(), "Failed to get current directory")
-		test_utils.WriteLogFile(specReport, GinkgoT().Name(), filepath.Join(currentDir, testLogsDirectory))
+		testutil.WriteLogFile(specReport, GinkgoT().Name(), filepath.Join(currentDir, testLogsDirectory))
 	} else {
 		log.Printf("Test passed")
 	}