mirror of
https://github.com/go-gitea/gitea.git
synced 2026-04-09 04:25:50 +02:00
Repair duration display for bad stopped timestamps (#37121)
Workflow run, job, task, and step durations could show **negative** values (e.g. `-50s`) when `Stopped` was missing, zero (epoch), or **before** `Started` (clock skew, races, reruns). The UI used `calculateDuration` with no validation. This change: - Uses each row`s **Updated** timestamp as a **fallback end time** when `Stopped` is invalid but the status is terminal, so duration approximates elapsed time instead of `0s` or a negative. - Keeps **`ActionRun.Duration()`** clamped to **≥ 0** when `PreviousDuration` plus the current segment would still be negative (legacy bad data). Fixes #34582. Co-authored-by: Composer <composer@cursor.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
parent
ff777cd2ad
commit
fc23bd7b3a
@ -153,7 +153,11 @@ func (run *ActionRun) LoadRepo(ctx context.Context) error {
|
||||
}
|
||||
|
||||
func (run *ActionRun) Duration() time.Duration {
|
||||
return calculateDuration(run.Started, run.Stopped, run.Status) + run.PreviousDuration
|
||||
d := calculateDuration(run.Started, run.Stopped, run.Status, run.Updated) + run.PreviousDuration
|
||||
if d < 0 {
|
||||
return 0
|
||||
}
|
||||
return d
|
||||
}
|
||||
|
||||
func (run *ActionRun) GetPushEventPayload() (*api.PushPayload, error) {
|
||||
|
||||
@ -72,7 +72,7 @@ func init() {
|
||||
}
|
||||
|
||||
func (job *ActionRunJob) Duration() time.Duration {
|
||||
return calculateDuration(job.Started, job.Stopped, job.Status)
|
||||
return calculateDuration(job.Started, job.Stopped, job.Status, job.Updated)
|
||||
}
|
||||
|
||||
func (job *ActionRunJob) LoadRun(ctx context.Context) error {
|
||||
|
||||
@ -5,10 +5,12 @@ package actions
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"code.gitea.io/gitea/models/db"
|
||||
repo_model "code.gitea.io/gitea/models/repo"
|
||||
"code.gitea.io/gitea/models/unittest"
|
||||
"code.gitea.io/gitea/modules/timeutil"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
@ -33,3 +35,13 @@ func TestUpdateRepoRunsNumbers(t *testing.T) {
|
||||
assert.Equal(t, 5, repo.NumActionRuns)
|
||||
assert.Equal(t, 3, repo.NumClosedActionRuns)
|
||||
}
|
||||
|
||||
func TestActionRun_Duration_NonNegative(t *testing.T) {
|
||||
run := &ActionRun{
|
||||
Started: timeutil.TimeStamp(100),
|
||||
Stopped: timeutil.TimeStamp(200),
|
||||
Status: StatusSuccess,
|
||||
PreviousDuration: -time.Hour,
|
||||
}
|
||||
assert.Equal(t, time.Duration(0), run.Duration())
|
||||
}
|
||||
|
||||
@ -77,7 +77,7 @@ func init() {
|
||||
}
|
||||
|
||||
func (task *ActionTask) Duration() time.Duration {
|
||||
return calculateDuration(task.Started, task.Stopped, task.Status)
|
||||
return calculateDuration(task.Started, task.Stopped, task.Status, task.Updated)
|
||||
}
|
||||
|
||||
func (task *ActionTask) IsStopped() bool {
|
||||
|
||||
@ -28,7 +28,7 @@ type ActionTaskStep struct {
|
||||
}
|
||||
|
||||
func (step *ActionTaskStep) Duration() time.Duration {
|
||||
return calculateDuration(step.Started, step.Stopped, step.Status)
|
||||
return calculateDuration(step.Started, step.Stopped, step.Status, step.Updated)
|
||||
}
|
||||
|
||||
func init() {
|
||||
|
||||
@ -13,6 +13,7 @@ import (
|
||||
"time"
|
||||
|
||||
auth_model "code.gitea.io/gitea/models/auth"
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
"code.gitea.io/gitea/modules/timeutil"
|
||||
"code.gitea.io/gitea/modules/util"
|
||||
)
|
||||
@ -72,13 +73,25 @@ func (indexes *LogIndexes) ToDB() ([]byte, error) {
|
||||
|
||||
var timeSince = time.Since
|
||||
|
||||
func calculateDuration(started, stopped timeutil.TimeStamp, status Status) time.Duration {
|
||||
// calculateDuration computes wall time for a run, job, task, or step. When status is terminal
|
||||
// but stopped is missing or inconsistent with started, fallbackEnd (typically the row Updated
|
||||
// time) is used so duration still reflects approximate elapsed time instead of 0 or a negative.
|
||||
func calculateDuration(started, stopped timeutil.TimeStamp, status Status, fallbackEnd timeutil.TimeStamp) time.Duration {
|
||||
if started == 0 {
|
||||
return 0
|
||||
}
|
||||
s := started.AsTime()
|
||||
if status.IsDone() {
|
||||
return stopped.AsTime().Sub(s)
|
||||
end := stopped
|
||||
if stopped.IsZero() || stopped < started {
|
||||
if !fallbackEnd.IsZero() && fallbackEnd >= started {
|
||||
end = fallbackEnd
|
||||
} else {
|
||||
log.Trace("actions: invalid duration timestamps (started=%d, stopped=%d, fallbackEnd=%d, status=%s)", started, stopped, fallbackEnd, status)
|
||||
return 0
|
||||
}
|
||||
}
|
||||
return end.AsTime().Sub(s)
|
||||
}
|
||||
return timeSince(s).Truncate(time.Second)
|
||||
}
|
||||
|
||||
@ -45,9 +45,10 @@ func Test_calculateDuration(t *testing.T) {
|
||||
return timeutil.TimeStamp(1000).AsTime().Sub(t)
|
||||
}
|
||||
type args struct {
|
||||
started timeutil.TimeStamp
|
||||
stopped timeutil.TimeStamp
|
||||
status Status
|
||||
started timeutil.TimeStamp
|
||||
stopped timeutil.TimeStamp
|
||||
status Status
|
||||
fallbackEnd timeutil.TimeStamp
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
@ -81,10 +82,48 @@ func Test_calculateDuration(t *testing.T) {
|
||||
},
|
||||
want: 100 * time.Second,
|
||||
},
|
||||
{
|
||||
name: "done_stopped_zero_no_fallback",
|
||||
args: args{
|
||||
started: 500,
|
||||
stopped: 0,
|
||||
status: StatusSuccess,
|
||||
},
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "done_stopped_zero_uses_fallback",
|
||||
args: args{
|
||||
started: 500,
|
||||
stopped: 0,
|
||||
status: StatusSuccess,
|
||||
fallbackEnd: 600,
|
||||
},
|
||||
want: 100 * time.Second,
|
||||
},
|
||||
{
|
||||
name: "done_stopped_before_started_no_fallback",
|
||||
args: args{
|
||||
started: 600,
|
||||
stopped: 550,
|
||||
status: StatusSuccess,
|
||||
},
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "done_stopped_before_started_uses_fallback",
|
||||
args: args{
|
||||
started: 600,
|
||||
stopped: 550,
|
||||
status: StatusSuccess,
|
||||
fallbackEnd: 650,
|
||||
},
|
||||
want: 50 * time.Second,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equalf(t, tt.want, calculateDuration(tt.args.started, tt.args.stopped, tt.args.status), "calculateDuration(%v, %v, %v)", tt.args.started, tt.args.stopped, tt.args.status)
|
||||
assert.Equalf(t, tt.want, calculateDuration(tt.args.started, tt.args.stopped, tt.args.status, tt.args.fallbackEnd), "calculateDuration(%v, %v, %v, %v)", tt.args.started, tt.args.stopped, tt.args.status, tt.args.fallbackEnd)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user