Cleanup go commands

Author: kalverra

tools/flakeguard/cmd/find.go

@@ -94,11 +94,6 @@ func init() {
 }
 
 func findAffectedPackages(baseRef, projectPath string, excludes []string, levels int) []string {
-	goList, err := golang.GoList()
-	if err != nil {
-		log.Error().Err(err).Msg("Error getting go list")
-		os.Exit(ErrorExitCode)
-	}
 	gitDiff, err := git.Diff(baseRef)
 	if err != nil {
 		log.Error().Err(err).Msg("Error getting the git diff")
@@ -110,7 +105,7 @@ func findAffectedPackages(baseRef, projectPath string, excludes []string, levels
 		os.Exit(ErrorExitCode)
 	}
 
-	packages, err := golang.ParsePackages(goList.Stdout)
+	packages, err := golang.Packages(projectPath)
 	if err != nil {
 		log.Error().Err(err).Msg("Error parsing packages")
 		os.Exit(ErrorExitCode)

tools/flakeguard/cmd/make_pr.go

@@ -2,17 +2,22 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/google/go-github/v72/github"
-	"github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/golang"
-	"github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/localdb"
 	"github.com/spf13/cobra"
 	"golang.org/x/oauth2"
+
+	flake_git "github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/git"
+	"github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/golang"
+	"github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/localdb"
 )
 
 var (
@@ -29,55 +34,118 @@ var MakePRCmd = &cobra.Command{
 func makePR(cmd *cobra.Command, args []string) error {
 	repo, err := git.PlainOpen(repoPath)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to open repo: %w", err)
 	}
 
 	db, err := localdb.LoadDBWithPath(localDBPath)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to load local db: %w", err)
 	}
 
 	currentlyFlakyEntries := db.GetAllCurrentlyFlakyEntries()
 
-	branchName := fmt.Sprintf("flakeguard-skip-%s", time.Now().Format("20060102150405"))
+	owner, repoName, defaultBranch, err := flake_git.GetOwnerRepoDefaultBranchFromLocalRepo(repoPath)
+	if err != nil {
+		return fmt.Errorf("failed to get repo info: %w", err)
+	}
+
 	targetRepoWorktree, err := repo.Worktree()
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to open repo's worktree: %w", err)
 	}
+
+	// First checkout default branch and pull latest
+	err = targetRepoWorktree.Checkout(&git.CheckoutOptions{
+		Branch: plumbing.NewBranchReferenceName(defaultBranch),
+	})
+	if err != nil {
+		if errors.Is(err, git.ErrUnstagedChanges) {
+			fmt.Println("Local repo has unstaged changes, please commit or stash them before running this command")
+		}
+		return fmt.Errorf("failed to checkout default branch %s: %w", defaultBranch, err)
+	}
+
+	fmt.Print("Fetching latest changes from default branch, tap your yubikey if it's blinking...")
+	err = repo.Fetch(&git.FetchOptions{})
+	if err != nil && err != git.NoErrAlreadyUpToDate {
+		return fmt.Errorf("failed to fetch latest: %w", err)
+	}
+	fmt.Println(" ✅")
+
+	fmt.Print("Pulling latest changes from default branch, tap your yubikey if it's blinking...")
+	err = targetRepoWorktree.Pull(&git.PullOptions{})
+	if err != nil && err != git.NoErrAlreadyUpToDate {
+		return fmt.Errorf("failed to pull latest changes: %w", err)
+	}
+	fmt.Println(" ✅")
+
+	// Create and checkout new branch
+	branchName := fmt.Sprintf("flakeguard-skip-%s", time.Now().Format("20060102150405"))
 	err = targetRepoWorktree.Checkout(&git.CheckoutOptions{
 		Branch: plumbing.NewBranchReferenceName(branchName),
 		Create: true,
 	})
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to checkout new branch: %w", err)
+	}
+
+	cleanUpBranch := true
+	defer func() {
+		if cleanUpBranch {
+			fmt.Printf("Cleaning up branch %s...", branchName)
+			err = targetRepoWorktree.Checkout(&git.CheckoutOptions{
+				Branch: plumbing.NewBranchReferenceName(defaultBranch),
+			})
+			if err != nil {
+				fmt.Printf("Failed to clean up branch: %v\n", err)
+			}
+			err = repo.Storer.RemoveReference(plumbing.NewBranchReferenceName(branchName))
+			if err != nil {
+				fmt.Printf("Failed to remove branch: %v\n", err)
+			}
+			fmt.Println(" ✅")
+		}
+	}()
+
+	if len(currentlyFlakyEntries) == 0 {
+		fmt.Println("No flaky tests found!")
+		return nil
 	}
 
-	testsToSkip := []golang.SkipTest{}
+	jiraTickets := []string{}
+	testsToSkip := []*golang.SkipTest{}
 	for _, entry := range currentlyFlakyEntries {
-		testsToSkip = append(testsToSkip, golang.SkipTest{
-			Package: entry.TestPackage,
-			Name:    entry.TestName,
+		testsToSkip = append(testsToSkip, &golang.SkipTest{
+			Package:    entry.TestPackage,
+			Name:       entry.TestName,
+			JiraTicket: entry.JiraTicket,
 		})
+		jiraTickets = append(jiraTickets, entry.JiraTicket)
 	}
 
 	err = golang.SkipTests(repoPath, testsToSkip)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to modify code to skip tests: %w", err)
 	}
 
 	_, err = targetRepoWorktree.Add(".")
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to add changes: %w", err)
 	}
-	_, err = targetRepoWorktree.Commit("Skips flaky tests", &git.CommitOptions{})
+
+	fmt.Print("Committing changes, tap your yubikey if it's blinking...")
+	commitHash, err := targetRepoWorktree.Commit(fmt.Sprintf("Skips flaky %d tests", len(testsToSkip)), &git.CommitOptions{})
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to commit changes: %w", err)
 	}
+	fmt.Println(" ✅")
 
+	fmt.Print("Pushing changes to remote, tap your yubikey if it's blinking...")
 	err = repo.Push(&git.PushOptions{})
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to push changes: %w", err)
 	}
+	fmt.Println(" ✅")
 
 	ctx := context.Background()
 	ts := oauth2.StaticTokenSource(
@@ -86,24 +154,71 @@ func makePR(cmd *cobra.Command, args []string) error {
 	tc := oauth2.NewClient(ctx, ts)
 	client := github.NewClient(tc)
 
-	owner := "your-org"
-	repoName := "your-repo"
+	var (
+		skippedTestsPRBody        strings.Builder
+		alreadySkippedTestsPRBody strings.Builder
+	)
+
+	for _, test := range testsToSkip {
+		if test.Skipped {
+			skippedTestsPRBody.WriteString(fmt.Sprintf("- Package: `%s`\n", test.Package))
+			skippedTestsPRBody.WriteString(fmt.Sprintf("  Test: `%s`\n", test.Name))
+			skippedTestsPRBody.WriteString(fmt.Sprintf("  Ticket: [%s](https://%s/browse/%s)\n", test.JiraTicket, os.Getenv("JIRA_DOMAIN"), test.JiraTicket))
+			skippedTestsPRBody.WriteString(fmt.Sprintf("  [View skip in PR](https://github.com/%s/%s/pull/%s/files#diff-%sL%d)\n\n", owner, repoName, branchName, commitHash, test.Line))
+		} else {
+			alreadySkippedTestsPRBody.WriteString(fmt.Sprintf("- Package: `%s`\n", test.Package))
+			alreadySkippedTestsPRBody.WriteString(fmt.Sprintf("  Test: `%s`\n", test.Name))
+			alreadySkippedTestsPRBody.WriteString(fmt.Sprintf("  Ticket: [%s](https://%s/browse/%s)\n", test.JiraTicket, os.Getenv("JIRA_DOMAIN"), test.JiraTicket))
+		}
+	}
+
 	pr := &github.NewPullRequest{
-		Title:               github.Ptr("Skip flaky tests"),
+		Title:               github.Ptr(fmt.Sprintf("[%s] Flakeguard: Skip flaky tests", strings.Join(jiraTickets, "] ["))),
 		Head:                github.Ptr(branchName),
-		Base:                github.Ptr("main"),
-		Body:                github.Ptr("This PR skips flaky tests."),
+		Base:                github.Ptr(defaultBranch),
+		Body:                github.Ptr(fmt.Sprintf("## Tests Skipped\n\n%s\n\n## Tests Already Skipped\n\n%s", skippedTestsPRBody.String(), alreadySkippedTestsPRBody.String())),
 		MaintainerCanModify: github.Ptr(true),
 	}
-	_, _, err = client.PullRequests.Create(ctx, owner, repoName, pr)
+
+	fmt.Println("PR Preview:")
+	fmt.Println("================================================")
+	fmt.Println(pr.Title)
+	fmt.Println("--------------------------------")
+	fmt.Printf("Merging '%s' into '%s'\n", branchName, defaultBranch)
+	fmt.Println(pr.Body)
+	fmt.Println("================================================")
+
+	fmt.Printf("To preview the code changes in the GitHub UI, visit: https://github.com/%s/%s/compare/%s...%s\n", owner, repoName, defaultBranch, branchName)
+	fmt.Print("Would you like to create the PR automatically from the CLI? (y/N): ")
+
+	var confirm string
+	_, err = fmt.Scanln(&confirm)
 	if err != nil {
 		return err
 	}
 
-	fmt.Println("PR created!")
+	if strings.ToLower(confirm) != "y" {
+		fmt.Println("Exiting. Please use the GitHub UI to create the PR.")
+		return nil
+	}
+
+	createdPR, resp, err := client.PullRequests.Create(ctx, owner, repoName, pr)
+	if err != nil {
+		return err
+	}
+	if resp.StatusCode != 201 {
+		body, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return fmt.Errorf("failed to read github response body while trying to create PR: %s\n%w", resp.Status, err)
+		}
+		return fmt.Errorf("failed to create PR, got bad status: %s\n%s", resp.Status, string(body))
+	}
+
+	cleanUpBranch = false
+	fmt.Printf("PR created! https://github.com/%s/%s/pull/%d\n", owner, repoName, createdPR.GetNumber())
 	return nil
 }
 
 func init() {
-	MakePRCmd.Flags().StringVarP(&repoPath, "repo", "r", ".", "Path to the repository to make the PR in")
+	MakePRCmd.Flags().StringVarP(&repoPath, "repoPath", "r", ".", "Local path to the repository to make the PR for")
 }

tools/flakeguard/git/git.go

@@ -9,6 +9,8 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/smartcontractkit/chainlink-testing-framework/tools/flakeguard/utils"
 )
 
@@ -186,3 +188,76 @@ func shouldExclude(excludes []string, item string) bool {
 	}
 	return false
 }
+
+// GetOwnerRepoDefaultBranchFromLocalRepo returns the owner, repo name, and default branch of a local git repository.
+// It uses the origin remote URL to determine the owner and repo name, and the default branch is determined from the
+// refs/remotes/origin/HEAD reference.
+func GetOwnerRepoDefaultBranchFromLocalRepo(repoPath string) (owner, repoName, defaultBranch string, err error) {
+	repo, err := git.PlainOpen(repoPath)
+	if err != nil {
+		return "", "", "", err
+	}
+
+	// Get remote URL (origin)
+	remotes, err := repo.Remotes()
+	if err != nil {
+		return "", "", "", err
+	}
+	var originURL string
+	for _, remote := range remotes {
+		if remote.Config().Name == "origin" && len(remote.Config().URLs) > 0 {
+			originURL = remote.Config().URLs[0]
+			break
+		}
+	}
+	if originURL == "" {
+		return "", "", "", fmt.Errorf("origin remote not found")
+	}
+
+	// Parse owner and repo from URL
+	originURL = strings.TrimSuffix(originURL, ".git")
+	var path string
+	if strings.Contains(originURL, "@github.com:") {
+		parts := strings.SplitN(originURL, ":", 2)
+		if len(parts) == 2 {
+			path = parts[1]
+		}
+	} else if strings.HasPrefix(originURL, "https://") {
+		parts := strings.SplitN(originURL, "github.com/", 2)
+		if len(parts) == 2 {
+			path = parts[1]
+		}
+	}
+	if path == "" {
+		return "", "", "", fmt.Errorf("could not parse remote URL: %s", originURL)
+	}
+	segments := strings.Split(path, "/")
+	if len(segments) != 2 {
+		return "", "", "", fmt.Errorf("unexpected path format: %s", path)
+	}
+	owner, repoName = segments[0], segments[1]
+
+	// Find default branch from refs/remotes/origin/HEAD
+	refs, err := repo.References()
+	if err != nil {
+		return "", "", "", err
+	}
+	err = refs.ForEach(func(ref *plumbing.Reference) error {
+		if ref.Name().IsRemote() && ref.Name().String() == "refs/remotes/origin/HEAD" {
+			target := ref.Target().String()
+			parts := strings.Split(target, "/")
+			if len(parts) > 0 {
+				defaultBranch = parts[len(parts)-1]
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return "", "", "", err
+	}
+	if defaultBranch == "" {
+		return "", "", "", fmt.Errorf("could not determine default branch")
+	}
+
+	return owner, repoName, defaultBranch, nil
+}