diff --git a/hack/linear-sync/linear.go b/hack/linear-sync/linear.go index 389c49b62e..10f4b202b0 100644 --- a/hack/linear-sync/linear.go +++ b/hack/linear-sync/linear.go @@ -39,6 +39,24 @@ func NewLinearClient(ctx context.Context, token string) LinearClient { return LinearClient{client: client} } +// isStableRelease checks if a version is a stable release (no pre-release suffix). +// Returns true for stable releases like v0.26.1, v4.5.0 +// Returns false for pre-releases like v0.26.1-alpha.1, v0.26.1-rc.4, v4.5.0-beta.2 +func isStableRelease(version string) bool { + // Remove 'v' prefix if present + version = strings.TrimPrefix(version, "v") + + // Check for pre-release suffixes + preReleaseSuffixes := []string{"-alpha", "-beta", "-rc", "-dev", "-pre", "-next"} + for _, suffix := range preReleaseSuffixes { + if strings.Contains(version, suffix) { + return false + } + } + + return true +} + // WorkflowStateID returns the ID of the a workflow state for the given team. func (l *LinearClient) WorkflowStateID(ctx context.Context, stateName, linearTeamName string) (string, error) { var query struct { @@ -115,6 +133,7 @@ func (l *LinearClient) IsIssueInStateByName(ctx context.Context, issueID string, // MoveIssueToState moves the issue to the given state if it's not already there. // It also adds a comment to the issue about when it was first released and on which tag. +// For stable releases on already-released issues, it adds a "now available in stable" comment. func (l *LinearClient) MoveIssueToState(ctx context.Context, dryRun bool, issueID, releasedStateID, readyForReleaseStateName, releaseTagName, releaseDate string) error { // (ThomasK33): Skip CVEs if strings.HasPrefix(strings.ToLower(issueID), "cve") { @@ -123,31 +142,51 @@ func (l *LinearClient) MoveIssueToState(ctx context.Context, dryRun bool, issueI logger := ctx.Value(LoggerKey).(*slog.Logger) + isStable := isStableRelease(releaseTagName) + currentIssueStateID, currentIssueStateName, err := l.IssueStateDetails(ctx, issueID) if err != nil { return fmt.Errorf("get issue state details: %w", err) } - if currentIssueStateID == releasedStateID { - logger.Debug("Issue already has desired state", "issueID", issueID, "stateID", releasedStateID) - return nil - } - - // Skip issues not in ready for release state - if currentIssueStateName != readyForReleaseStateName { - logger.Debug("Skipping issue not in ready for release state", "issueID", issueID, "currentState", currentIssueStateName, "requiredState", readyForReleaseStateName) - return nil - } + alreadyReleased := currentIssueStateID == releasedStateID - if !dryRun { - if err := l.updateIssueState(ctx, issueID, releasedStateID); err != nil { - return fmt.Errorf("update issue state: %w", err) + // If already in released state: + // - Pre-releases: skip entirely (already released in a previous pre-release) + // - Stable releases: skip state update but add "now available in stable" comment + if alreadyReleased { + if !isStable { + logger.Debug("Issue already has desired state", "issueID", issueID, "stateID", releasedStateID) + return nil } + logger.Debug("Issue already released, adding stable release comment", "issueID", issueID) } else { - logger.Info("Would update issue state", "issueID", issueID, "releasedStateID", releasedStateID) + // Skip issues not in ready for release state + if currentIssueStateName != readyForReleaseStateName { + logger.Debug("Skipping issue not in ready for release state", "issueID", issueID, "currentState", currentIssueStateName, "requiredState", readyForReleaseStateName) + return nil + } + + // Update issue state to Released + if !dryRun { + if err := l.updateIssueState(ctx, issueID, releasedStateID); err != nil { + return fmt.Errorf("update issue state: %w", err) + } + } else { + logger.Info("Would update issue state", "issueID", issueID, "releasedStateID", releasedStateID) + } + logger.Info("Moved issue to desired state", "issueID", issueID, "stateID", releasedStateID) } - releaseComment := fmt.Sprintf("This issue was first released in %v on %v", releaseTagName, releaseDate) + // Add release comment + // Use different text for stable releases on already-released issues to avoid + // confusion with the "first released in" pattern used by linear-webhook-service + var releaseComment string + if alreadyReleased && isStable { + releaseComment = fmt.Sprintf("Now available in stable release %v (released %v)", releaseTagName, releaseDate) + } else { + releaseComment = fmt.Sprintf("This issue was first released in %v on %v", releaseTagName, releaseDate) + } if !dryRun { if err := l.createComment(ctx, issueID, releaseComment); err != nil { @@ -157,8 +196,6 @@ func (l *LinearClient) MoveIssueToState(ctx context.Context, dryRun bool, issueI logger.Info("Would create comment on issue", "issueID", issueID, "comment", releaseComment) } - logger.Info("Moved issue to desired state", "issueID", issueID, "stateID", releasedStateID) - return nil } @@ -201,4 +238,3 @@ func (l *LinearClient) createComment(ctx context.Context, issueID, releaseCommen return nil } - diff --git a/hack/linear-sync/linear_test.go b/hack/linear-sync/linear_test.go index f6fbba2bcc..e3ac290e06 100644 --- a/hack/linear-sync/linear_test.go +++ b/hack/linear-sync/linear_test.go @@ -54,9 +54,9 @@ func TestMoveIssueLogic(t *testing.T) { // MockLinearClient is a mock implementation of the LinearClient interface for testing type MockLinearClient struct { - mockIssueStates map[string]string - mockIssueStateNames map[string]string - mockWorkflowIDs map[string]string + mockIssueStates map[string]string + mockIssueStateNames map[string]string + mockWorkflowIDs map[string]string } func NewMockLinearClient() *MockLinearClient { @@ -109,25 +109,25 @@ func (m *MockLinearClient) MoveIssueToState(ctx context.Context, dryRun bool, is if strings.HasPrefix(strings.ToLower(issueID), "cve") { return nil } - + currentStateID, currentStateName, _ := m.IssueStateDetails(ctx, issueID) - + // Already in released state if currentStateID == releasedStateID { return nil } - + // Skip if not in ready for release state if currentStateName != readyForReleaseStateName { return fmt.Errorf("issue %s not in ready for release state", issueID) } - + // Only ENG-1234 is expected to be moved successfully // Explicitly return errors for other issues to ensure the test only counts ENG-1234 if issueID != "ENG-1234" { return fmt.Errorf("would not move issue %s for test purposes", issueID) } - + return nil } @@ -136,8 +136,8 @@ func TestIsIssueInState(t *testing.T) { ctx := context.Background() testCases := []struct { - IssueID string - StateID string + IssueID string + StateID string ExpectedResult bool }{ {"ENG-1234", "ready-state-id", true}, @@ -164,10 +164,10 @@ func TestMoveIssueStateFiltering(t *testing.T) { // Create a custom mock client for this test mockClient := &MockLinearClient{ mockIssueStates: map[string]string{ - "ENG-1234": "ready-state-id", // Ready for release - "ENG-5678": "in-progress-id", // In progress - "ENG-9012": "released-id", // Already released - "CVE-1234": "ready-state-id", // Ready but should be skipped as CVE + "ENG-1234": "ready-state-id", // Ready for release + "ENG-5678": "in-progress-id", // In progress + "ENG-9012": "released-id", // Already released + "CVE-1234": "ready-state-id", // Ready but should be skipped as CVE }, mockIssueStateNames: map[string]string{ "ENG-1234": "Ready for Release", @@ -181,7 +181,7 @@ func TestMoveIssueStateFiltering(t *testing.T) { "In Progress": "in-progress-id", }, } - + ctx := context.Background() // Test cases for the overall filtering logic @@ -198,19 +198,19 @@ func TestMoveIssueStateFiltering(t *testing.T) { if strings.HasPrefix(strings.ToLower(issueID), "cve") { continue } - + currentStateID, currentStateName, _ := mockClient.IssueStateDetails(ctx, issueID) - + // Skip if already in released state if currentStateID == releasedStateID { continue } - + // Skip if not in ready for release state if currentStateName != readyForReleaseStateName { continue } - + // This issue would be moved actualMoved = append(actualMoved, issueID) } @@ -230,7 +230,7 @@ func TestMoveIssueStateFiltering(t *testing.T) { break } } - + if !found { t.Errorf("Expected issue %s to be moved, but it wasn't in the result set", expectedID) } @@ -243,12 +243,12 @@ func TestIssueIDsExtraction(t *testing.T) { defer func() { issuesInBodyREs = originalRegex }() - + // For testing, use a regex that matches any 3-letter prefix format issuesInBodyREs = []*regexp.Regexp{ regexp.MustCompile(`(?P\w{3}-\d{4})`), } - + testCases := []struct { name string body string @@ -286,7 +286,7 @@ func TestIssueIDsExtraction(t *testing.T) { expected: []string{}, }, } - + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { pr := LinearPullRequest{ @@ -295,15 +295,15 @@ func TestIssueIDsExtraction(t *testing.T) { HeadRefName: tc.headRefName, }, } - + result := pr.IssueIDs() - + if len(result) != len(tc.expected) { t.Errorf("Expected %d issues, got %d", len(tc.expected), len(result)) t.Errorf("Expected: %v, Got: %v", tc.expected, result) return } - + // Check all expected IDs are found (ignoring order) for _, expectedID := range tc.expected { found := false @@ -320,3 +320,90 @@ func TestIssueIDsExtraction(t *testing.T) { }) } } + +func TestIsStableRelease(t *testing.T) { + testCases := []struct { + version string + expected bool + }{ + // Stable releases + {"v0.26.1", true}, + {"v4.5.0", true}, + {"v1.0.0", true}, + {"0.26.1", true}, // without v prefix + {"v27.0.0", true}, + + // Pre-releases + {"v0.26.1-alpha.1", false}, + {"v0.26.1-alpha.5", false}, + {"v0.26.1-beta.1", false}, + {"v0.26.1-rc.1", false}, + {"v0.26.1-rc.4", false}, + {"v0.26.1-dev.1", false}, + {"v0.26.1-pre.1", false}, + {"v0.26.1-next.1", false}, + {"v4.5.0-beta.2", false}, + {"0.27.0-alpha.1", false}, // without v prefix + } + + for _, tc := range testCases { + t.Run(tc.version, func(t *testing.T) { + result := isStableRelease(tc.version) + if result != tc.expected { + t.Errorf("isStableRelease(%q) = %v, want %v", tc.version, result, tc.expected) + } + }) + } +} + +func TestStableReleaseCommentText(t *testing.T) { + // Test the comment text logic for different scenarios + testCases := []struct { + name string + alreadyReleased bool + isStable bool + releaseTag string + releaseDate string + expectedContains string + }{ + { + name: "First release (pre-release)", + alreadyReleased: false, + isStable: false, + releaseTag: "v0.27.0-alpha.1", + releaseDate: "2025-01-15", + expectedContains: "first released in", + }, + { + name: "First release (stable)", + alreadyReleased: false, + isStable: true, + releaseTag: "v0.27.0", + releaseDate: "2025-02-01", + expectedContains: "first released in", + }, + { + name: "Stable release on already-released issue", + alreadyReleased: true, + isStable: true, + releaseTag: "v0.27.0", + releaseDate: "2025-02-01", + expectedContains: "Now available in stable release", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var releaseComment string + if tc.alreadyReleased && tc.isStable { + releaseComment = fmt.Sprintf("Now available in stable release %v (released %v)", tc.releaseTag, tc.releaseDate) + } else { + releaseComment = fmt.Sprintf("This issue was first released in %v on %v", tc.releaseTag, tc.releaseDate) + } + + if !strings.Contains(releaseComment, tc.expectedContains) { + t.Errorf("Comment %q does not contain expected text %q", releaseComment, tc.expectedContains) + } + }) + } +}