Add reviews given last 30 days to the fast pr stats mode for mender-test-bot

pr_stats.go

@@ -76,17 +76,23 @@ type PRStatsConfig struct {
 }
 
 func loadPRStatsConfig(path string) (*PRStatsConfig, error) {
+	isDefaultPath := false
 	if path == "" {
 		path = os.Getenv("PR_STATS_CONFIG_PATH")
 		if path == "" {
 			path = "pr_stats_config.json"
+			isDefaultPath = true
 			if _, err := os.Stat(path); os.IsNotExist(err) {
 				path = "/pr_stats_config.json"
 			}
 		}
 	}
 	data, err := os.ReadFile(path)
 	if err != nil {
+		if isDefaultPath && os.IsNotExist(err) {
+			logrus.Infof("Optional config file %s not found, using defaults", path)
+			return nil, nil
+		}
 		logrus.Warnf("Could not read config file at %s: %s", path, err)
 		return nil, err
 	}
@@ -269,7 +275,7 @@ func getPRStats(
 	opts PRStatsOptions,
 ) (string, error) {
 	thirtyDaysAgo := time.Now().AddDate(0, 0, -30)
-	needReviews := opts.Mode == prStatsModeFull
+	needReviews := true
 
 	results := make([]repoResult, len(opts.Repos))
 
@@ -658,9 +664,9 @@ func writeReportTeamActivity(
 ) {
 	report.WriteString("\n### Team Activity\n")
 	if mode == prStatsModeTeam {
-		report.WriteString("| User | Opened (30d) | Closed (30d) | ")
+		report.WriteString("| User | Opened (30d) | Closed (30d) | Reviews (30d) | ")
 		report.WriteString("Median TTC | **Open Now** | **Assigned/Reviewing** |\n")
-		report.WriteString("|---|---|---|---|---|---|\n")
+		report.WriteString("|---|---|---|---|---|---|---|\n")
 	} else {
 		report.WriteString("| User | Opened (30d) | Closed (30d) | Reviews (30d) | ")
 		report.WriteString("Median TTC | Median TTRv | **Open Now** | **Assigned/Reviewing** |\n")
@@ -681,8 +687,8 @@ func writeReportTeamActivity(
 		_, ttcMed, _ := getStats(s.CloseTimes)
 		if mode == prStatsModeTeam {
 			report.WriteString(fmt.Sprintf(
-				"| %s | %d | %d | %s | **%d** | **%d** |\n",
-				s.Login, s.Opened, s.Closed, ttcMed,
+				"| %s | %d | %d | %d | %s | **%d** | **%d** |\n",
+				s.Login, s.Opened, s.Closed, s.Reviewed, ttcMed,
 				s.CurrentOpen, s.CurrentAssigned,
 			))
 		} else {

pr_stats_config.json

@@ -1,7 +1,7 @@
 {
   "global": {
     "excluded_users": ["mender-test-bot", "dependabot[bot]"],
-    "excluded_labels": ["dependencies", "onhold", "question", "autorelease: pending", "external contribution"],
+    "excluded_labels": ["dependencies", "onhold", "question", "autorelease: pending", "release:pending", "external contribution"],
     "exclude_drafts": true,
     "sla_hours": 48
   },

pr_stats_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"os"
 	"testing"
 	"time"
 
@@ -86,7 +87,7 @@ func TestGetPRStatsFull(t *testing.T) {
 	mclient.AssertExpectations(t)
 }
 
-func TestGetPRStatsTeamModeSkipsReviews(t *testing.T) {
+func TestGetPRStatsTeamModeIncludesReviews(t *testing.T) {
 	mclient := &mock_github.Client{}
 	ctx := context.Background()
 	org := "mendersoftware"
@@ -112,7 +113,14 @@ func TestGetPRStatsTeamModeSkipsReviews(t *testing.T) {
 		return opts.State == "closed"
 	})).Return([]*github.PullRequest{}, nil)
 
-	// ListReviews should NOT be called in team mode
+	// ListReviews SHOULD be called in team mode now
+	mclient.On("ListReviews", mock.Anything, org, repo, 1, mock.Anything).Return([]*github.PullRequestReview{
+		{
+			User:        &github.User{Login: github.String("reviewer1")},
+			SubmittedAt: &now,
+		},
+	}, nil)
+
 	opts := PRStatsOptions{
 		Repos: []string{repo},
 		Mode:  prStatsModeTeam,
@@ -123,12 +131,12 @@ func TestGetPRStatsTeamModeSkipsReviews(t *testing.T) {
 	assert.Contains(t, report, "Team Activity")
 	assert.NotContains(t, report, "Metrics Summary")
 	assert.NotContains(t, report, "PRs Needing Attention")
-	// Team mode should not show review columns
-	assert.NotContains(t, report, "Reviews (30d)")
+	// Team mode should show reviews column now
+	assert.Contains(t, report, "Reviews (30d)")
+	// But not Median TTRv (still only in full mode)
 	assert.NotContains(t, report, "Median TTRv")
 
 	mclient.AssertExpectations(t)
-	mclient.AssertNotCalled(t, "ListReviews", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestGetPRStatsExcludesUsers(t *testing.T) {
@@ -165,6 +173,9 @@ func TestGetPRStatsExcludesUsers(t *testing.T) {
 		return opts.State == "closed"
 	})).Return([]*github.PullRequest{}, nil)
 
+	// ListReviews MUST be mocked now as it is always called
+	mclient.On("ListReviews", mock.Anything, org, repo, 2, mock.Anything).Return([]*github.PullRequestReview{}, nil)
+
 	opts := PRStatsOptions{
 		Repos:         []string{repo},
 		Mode:          prStatsModeTeam,
@@ -213,6 +224,9 @@ func TestGetPRStatsExcludesDrafts(t *testing.T) {
 		return opts.State == "closed"
 	})).Return([]*github.PullRequest{}, nil)
 
+	// ListReviews MUST be mocked now as it is always called
+	mclient.On("ListReviews", mock.Anything, org, repo, 2, mock.Anything).Return([]*github.PullRequestReview{}, nil)
+
 	opts := PRStatsOptions{
 		Repos:         []string{repo},
 		Mode:          prStatsModeTeam,
@@ -254,6 +268,9 @@ func TestGetPRStatsMultiRepo(t *testing.T) {
 		mclient.On("ListPullRequests", mock.Anything, org, repo, mock.MatchedBy(func(opts *github.PullRequestListOptions) bool {
 			return opts.State == "closed"
 		})).Return([]*github.PullRequest{}, nil)
+
+		// ListReviews MUST be mocked now as it is always called
+		mclient.On("ListReviews", mock.Anything, org, repo, 1, mock.Anything).Return([]*github.PullRequestReview{}, nil)
 	}
 
 	opts := PRStatsOptions{
@@ -268,6 +285,7 @@ func TestGetPRStatsMultiRepo(t *testing.T) {
 	mclient.AssertExpectations(t)
 }
 
+
 func TestCalculateWorkingTime(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -466,4 +484,26 @@ func TestLoadPRStatsConfig(t *testing.T) {
 		assert.Error(t, err)
 		assert.Nil(t, config)
 	})
+
+	t.Run("returns nil for missing default file", func(t *testing.T) {
+		// Create a temporary directory and change to it
+		tmpDir, err := os.MkdirTemp("", "pr_stats_test")
+		assert.NoError(t, err)
+		defer os.RemoveAll(tmpDir)
+
+		oldWd, err := os.Getwd()
+		assert.NoError(t, err)
+		err = os.Chdir(tmpDir)
+		assert.NoError(t, err)
+		defer os.Chdir(oldWd)
+
+		// Ensure environment variable is not set
+		oldEnv := os.Getenv("PR_STATS_CONFIG_PATH")
+		os.Unsetenv("PR_STATS_CONFIG_PATH")
+		defer os.Setenv("PR_STATS_CONFIG_PATH", oldEnv)
+
+		config, err := loadPRStatsConfig("")
+		assert.NoError(t, err)
+		assert.Nil(t, config)
+	})
 }