Mirrors/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-07-21 01:38:30 +02:00
Code Issues Activity

refactor: extract buildRunOptions for better test coverage

Browse Source 
- Extract FindRunOptions construction logic from ListRuns into buildRunOptions function
- Update TestListRunsWorkflowFiltering and related tests to use actual production code
- Tests now verify production logic instead of duplicating parameter parsing
- Improved error handling by returning errors instead of direct HTTP responses
- Ensures future changes to parameter parsing are properly tested

This addresses the issue where tests were bypassing production code logic,
making them less reliable for catching regressions.
This commit is contained in:
Brice Ruth 2025-07-04 10:41:00 -05:00
parent 1b9b410c5d
commit 6b074cb3e9
No known key found for this signature in database
GPG Key ID: 5DFD569B02D44E21
2 changed files with 35 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
routers/api/v1/shared/action.go
View File

@ -139,10 +139,8 @@ func convertToInternal(s string) ([]actions_model.Status, error) {
// ownerID != 0 and repoID == 0 means all runs for the given user/org
// ownerID != 0 and repoID != 0 undefined behavior
// Access rights are checked at the API route level
func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
if ownerID != 0 && repoID != 0 {
setting.PanicInDevOrTesting("ownerID and repoID should not be both set")
}
// buildRunOptions builds the FindRunOptions from context parameters
func buildRunOptions(ctx *context.APIContext, ownerID, repoID int64) (actions_model.FindRunOptions, error) {
opts := actions_model.FindRunOptions{
OwnerID: ownerID,
RepoID: repoID,
@ -159,16 +157,14 @@ func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
for _, status := range ctx.FormStrings("status") {
values, err := convertToInternal(status)
if err != nil {
ctx.APIError(http.StatusBadRequest, fmt.Errorf("Invalid status %s", status))
return
return opts, fmt.Errorf("Invalid status %s", status)
}
opts.Status = append(opts.Status, values...)
}
if actor := ctx.FormString("actor"); actor != "" {
user, err := user_model.GetUserByName(ctx, actor)
if err != nil {
ctx.APIErrorInternal(err)
return
return opts, err
}
opts.TriggerUserID = user.ID
}
@ -249,6 +245,20 @@ func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
}
}
return opts, nil
}
func ListRuns(ctx *context.APIContext, ownerID, repoID int64) {
if ownerID != 0 && repoID != 0 {
setting.PanicInDevOrTesting("ownerID and repoID should not be both set")
}
opts, err := buildRunOptions(ctx, ownerID, repoID)
if err != nil {
ctx.APIError(http.StatusBadRequest, err)
return
}
runs, total, err := db.FindAndCount[actions_model.ActionRun](ctx, opts)
if err != nil {
ctx.APIErrorInternal(err)

89
routers/api/v1/shared/action_list_runs_test.go
View File

@ -7,7 +7,6 @@ import (
"testing"
"time"
actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/services/contexttest"
@ -30,12 +29,8 @@ func TestListRunsWorkflowFiltering(t *testing.T) {
// Test case 1: With workflow_id parameter (simulating /workflows/{workflow_id}/runs endpoint)
ctx.SetPathParam("workflow_id", "test-workflow-123")
// Simulate the FindRunOptions creation that happens in ListRuns
opts := actions_model.FindRunOptions{
OwnerID: 0,
RepoID: ctx.Repo.Repository.ID,
WorkflowID: ctx.PathParam("workflow_id"), // This is the key change being tested
}
opts, err := buildRunOptions(ctx, 0, ctx.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the WorkflowID is correctly extracted from path parameter
assert.Equal(t, "test-workflow-123", opts.WorkflowID)
@ -48,10 +43,8 @@ func TestListRunsWorkflowFiltering(t *testing.T) {
contexttest.LoadUser(t, ctx2, 2)
// No SetPathParam call - simulates general runs endpoint
opts2 := actions_model.FindRunOptions{
RepoID: ctx2.Repo.Repository.ID,
WorkflowID: ctx2.PathParam("workflow_id"),
}
opts2, err := buildRunOptions(ctx2, 0, ctx2.Repo.Repository.ID)
assert.NoError(t, err)
// Verify WorkflowID is empty when path parameter is not set
assert.Empty(t, opts2.WorkflowID)
@ -70,14 +63,9 @@ func TestListRunsExcludePullRequestsParam(t *testing.T) {
contexttest.LoadRepo(t, ctx, 1)
contexttest.LoadUser(t, ctx, 2)
// Call the actual parsing logic from ListRuns
opts := actions_model.FindRunOptions{
RepoID: ctx.Repo.Repository.ID,
}
if ctx.FormBool("exclude_pull_requests") {
opts.ExcludePullRequests = true
}
// Call the actual production logic
opts, err := buildRunOptions(ctx, 0, ctx.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the ExcludePullRequests is correctly set based on the form value
assert.True(t, opts.ExcludePullRequests)
@ -87,13 +75,8 @@ func TestListRunsExcludePullRequestsParam(t *testing.T) {
contexttest.LoadRepo(t, ctx2, 1)
contexttest.LoadUser(t, ctx2, 2)
opts2 := actions_model.FindRunOptions{
RepoID: ctx2.Repo.Repository.ID,
}
if ctx2.FormBool("exclude_pull_requests") {
opts2.ExcludePullRequests = true
}
opts2, err := buildRunOptions(ctx2, 0, ctx2.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the ExcludePullRequests is correctly set for "1" value
assert.True(t, opts2.ExcludePullRequests)
@ -103,13 +86,8 @@ func TestListRunsExcludePullRequestsParam(t *testing.T) {
contexttest.LoadRepo(t, ctx3, 1)
contexttest.LoadUser(t, ctx3, 2)
opts3 := actions_model.FindRunOptions{
RepoID: ctx3.Repo.Repository.ID,
}
if ctx3.FormBool("exclude_pull_requests") {
opts3.ExcludePullRequests = true
}
opts3, err := buildRunOptions(ctx3, 0, ctx3.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the ExcludePullRequests is NOT set for "false" value
assert.False(t, opts3.ExcludePullRequests)
@ -125,21 +103,8 @@ func TestListRunsCreatedParam(t *testing.T) {
contexttest.LoadRepo(t, ctx, 1)
contexttest.LoadUser(t, ctx, 2)
opts := actions_model.FindRunOptions{
RepoID: ctx.Repo.Repository.ID,
}
// Simulate the date parsing logic from ListRuns
if created := ctx.FormString("created"); created != "" {
if created == "2023-01-01..2023-12-31" {
startDate, _ := time.Parse("2006-01-02", "2023-01-01")
endDate, _ := time.Parse("2006-01-02", "2023-12-31")
endDate = endDate.Add(24*time.Hour - time.Second)
opts.CreatedAfter = startDate
opts.CreatedBefore = endDate
}
}
opts, err := buildRunOptions(ctx, 0, ctx.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the date range is correctly parsed
expectedStart, _ := time.Parse("2006-01-02", "2023-01-01")
@ -154,18 +119,8 @@ func TestListRunsCreatedParam(t *testing.T) {
contexttest.LoadRepo(t, ctx2, 1)
contexttest.LoadUser(t, ctx2, 2)
opts2 := actions_model.FindRunOptions{
RepoID: ctx2.Repo.Repository.ID,
}
// Simulate the date parsing logic for >= format
if created := ctx2.FormString("created"); created != "" {
if created == ">=2023-01-01" {
dateStr := "2023-01-01"
startDate, _ := time.Parse("2006-01-02", dateStr)
opts2.CreatedAfter = startDate
}
}
opts2, err := buildRunOptions(ctx2, 0, ctx2.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the date is correctly parsed
expectedStart2, _ := time.Parse("2006-01-02", "2023-01-01")
@ -177,18 +132,8 @@ func TestListRunsCreatedParam(t *testing.T) {
contexttest.LoadRepo(t, ctx3, 1)
contexttest.LoadUser(t, ctx3, 2)
opts3 := actions_model.FindRunOptions{
RepoID: ctx3.Repo.Repository.ID,
}
// Simulate the date parsing logic for exact date
if created := ctx3.FormString("created"); created != "" {
if created == "2023-06-15" {
exactDate, _ := time.Parse("2006-01-02", created)
opts3.CreatedAfter = exactDate
opts3.CreatedBefore = exactDate.Add(24*time.Hour - time.Second)
}
}
opts3, err := buildRunOptions(ctx3, 0, ctx3.Repo.Repository.ID)
assert.NoError(t, err)
// Verify the exact date is correctly parsed to a date range
exactDate, _ := time.Parse("2006-01-02", "2023-06-15")