Say max_retries is the total attempt count, not a retry count (closes #316)
check / check (push) Successful in 3m54s
check / check (push) Successful in 3m54s
The help text under the field on both target forms and the max_retries rows in the README now say the number is the total number of delivery attempts: 0 is a single attempt with no retries and no circuit breaker, and N is N attempts in total. The delivery code already worked this way; only the wording was wrong, so an operator wanting one try plus two retries would have entered 2 instead of 3. A UI copy test renders both forms and pins the wording. Delivery behaviour is unchanged. Model: opus-4-8 (implementation); fable-5-1 (merge)
This commit was merged in pull request #320.
This commit is contained in:
@@ -300,3 +300,80 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
|
||||
"the page must render to completion, not abort partway",
|
||||
)
|
||||
}
|
||||
|
||||
// maxRetriesHelp is the wording both target forms must carry. The
|
||||
// delivery core makes max_retries attempts in total, not that many
|
||||
// retries on top of a first try (a fresh delivery starts at attempt 1
|
||||
// and target_http gives up once the attempt number reaches
|
||||
// max_retries), and 0 is special-cased to a single fire-and-forget
|
||||
// attempt with no circuit breaker.
|
||||
const maxRetriesHelp = "This is the total number of delivery attempts, " +
|
||||
"not retries on top of the first: a value of 3 makes three attempts " +
|
||||
"in all. 0 means a single attempt with no retries and no circuit " +
|
||||
"breaker."
|
||||
|
||||
// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
|
||||
// help text on both the create form (the add-target form on the webhook
|
||||
// detail page) and the edit form, so the copy cannot drift back to
|
||||
// calling the number a retry count.
|
||||
func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var h *handlers.Handlers
|
||||
|
||||
var sess *session.Session
|
||||
|
||||
app := newTestApp(t, &h, &sess)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
|
||||
webhook.ID = testWebhookID
|
||||
|
||||
entrypoint := database.Entrypoint{Path: "abc123"}
|
||||
entrypoint.ID = "ep-1"
|
||||
|
||||
createBody := renderPage(
|
||||
t, h, sess, "source_detail.html", map[string]any{
|
||||
dataKeyWebhook: webhook,
|
||||
"Entrypoints": handlers.NewEntrypointViews(
|
||||
[]database.Entrypoint{entrypoint},
|
||||
),
|
||||
"Targets": delivery.NewTargetViews(nil),
|
||||
"Events": []database.Event{},
|
||||
"BaseURL": "https://hooks.example.com",
|
||||
},
|
||||
)
|
||||
|
||||
assert.Contains(
|
||||
t, createBody, maxRetriesHelp,
|
||||
"the add-target form must explain max_retries as total attempts",
|
||||
)
|
||||
|
||||
// A slack target exercises the same max_retries field while needing
|
||||
// only Config.URL from the edit template, so the test data stays
|
||||
// minimal. The Target key mirrors the field names the template reads
|
||||
// off the handler's view value.
|
||||
editBody := renderPage(
|
||||
t, h, sess, "target_edit.html", map[string]any{
|
||||
dataKeyWebhook: webhook,
|
||||
"Target": map[string]any{
|
||||
"ID": "tg-1",
|
||||
"Name": "t",
|
||||
"Type": "slack",
|
||||
"Active": true,
|
||||
"MaxRetries": 3,
|
||||
"Config": map[string]any{
|
||||
"URL": "https://hooks.slack.com/services/x",
|
||||
},
|
||||
},
|
||||
dataKeyError: "",
|
||||
},
|
||||
)
|
||||
|
||||
assert.Contains(
|
||||
t, editBody, maxRetriesHelp,
|
||||
"the target edit form must explain max_retries as total attempts",
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user