Skip to content

Commit 251ab64

Browse files
committed
Fix tests and updates
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
1 parent 29d3ca8 commit 251ab64

File tree

4 files changed

+51
-42
lines changed

4 files changed

+51
-42
lines changed

pkg/msg/message.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func (m *Message) BuildComment(
254254
}
255255

256256
// App header: show emoji only if state is not None
257-
appHeader := "\n---\n\n<details>\n<summary>\n\n"
257+
appHeader := "\n\n<details>\n<summary>\n\n"
258258
if appState == pkg.StateNone {
259259
appHeader += fmt.Sprintf("## ArgoCD Application Checks: `%s`\n", appName)
260260
} else {
@@ -297,8 +297,6 @@ func (m *Message) BuildComment(
297297
if contentLength+len(summary) > maxContentLength {
298298
appendChunk()
299299
}
300-
sb.WriteString(summary)
301-
contentLength += len(summary)
302300

303301
// Details block (may need to split across chunks)
304302
msg := fmt.Sprintf("<details>\n<summary>%s</summary>\n\n%s\n</details>", summary, check.Details)
@@ -330,8 +328,8 @@ func (m *Message) BuildComment(
330328
}
331329

332330
// Close the app details block
333-
sb.WriteString("</details>")
334-
contentLength += len("</details>")
331+
sb.WriteString("</details>\n\n")
332+
contentLength += len("</details>\n\n")
335333

336334
updateWritten = true
337335
}

pkg/msg/message_test.go

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,23 @@ func TestBuildComment(t *testing.T) {
3434
m := NewMessage("message", 1, 2, fakeEmojiable{":test:"})
3535
m.apps = appResults
3636
comment := m.BuildComment(context.TODO(), time.Now(), "commit-sha", "label-filter", false, "test-identifier", 1, 2, 1000, "https://github.com/zapier/kubechecks/pull/1")
37-
assert.Equal(t, []string{`## Kubechecks test-identifier Report
37+
assert.Equal(t, []string{`# Kubechecks test-identifier Report
3838
39-
---
4039
4140
<details>
4241
<summary>
4342
4443
## ArgoCD Application Checks: ` + "`myapp`" + ` :test:
4544
</summary>
4645
47-
this failed bigly Error :test:<details>
46+
<details>
4847
<summary>this failed bigly Error :test:</summary>
4948
5049
should add some important details here
5150
</details></details>
5251
52+
53+
5354
<small> _Done. CommitSHA: commit-sha_ <small>
5455
`}, comment)
5556
}
@@ -86,35 +87,37 @@ func TestBuildComment_SkipUnchanged(t *testing.T) {
8687
m.apps = appResults
8788
comment := m.BuildComment(context.TODO(), time.Now(), "commit-sha", "label-filter", false, "test-identifier", 1, 2, 1000, "https://github.com/zapier/kubechecks/pull/1")
8889

89-
expected := `## Kubechecks test-identifier Report
90+
expected := `# Kubechecks test-identifier Report
9091
91-
---
9292
9393
<details>
9494
<summary>
9595
9696
## ArgoCD Application Checks: ` + "`myapp`" + ` :test:
9797
</summary>
9898
99-
this failed bigly Error :test:<details>
99+
<details>
100100
<summary>this failed bigly Error :test:</summary>
101101
102102
should add some important details here
103103
</details></details>
104-
---
104+
105+
105106
106107
<details>
107108
<summary>
108109
109110
## ArgoCD Application Checks: ` + "`myapp2`" + ` :test:
110111
</summary>
111112
112-
this thing failed Error :test:<details>
113+
<details>
113114
<summary>this thing failed Error :test:</summary>
114115
115116
should add some important details here
116117
</details></details>
117118
119+
120+
118121
<small> _Done. CommitSHA: commit-sha_ <small>
119122
`
120123
// Accept either with or without the extra closing tag before the footer
@@ -252,7 +255,7 @@ func TestBuildComment_Deep(t *testing.T) {
252255
assert.Contains(t, comments[0], "app1")
253256
assert.Contains(t, comments[0], "all good")
254257
assert.Contains(t, comments[0], "details")
255-
assert.Contains(t, comments[0], "## Kubechecks id Report")
258+
assert.Contains(t, comments[0], "# Kubechecks id Report")
256259
assert.Contains(t, comments[0], "_Done. CommitSHA: sha_")
257260
})
258261

@@ -293,21 +296,29 @@ func TestBuildComment_Deep(t *testing.T) {
293296
comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 950, "prlink")
294297
require.Greater(t, len(comments), 1)
295298
foundSplitWarning := false
296-
foundFirstPart := false
297-
foundSecondPart := false
299+
foundDetails := false
298300
for _, c := range comments {
299301
if strings.Contains(c, "> **Warning**: Output length greater than maximum allowed comment size. Continued in next comment") {
300302
foundSplitWarning = true
301303
}
302-
if strings.Contains(c, longSummary[:800]) {
304+
if strings.Contains(c, ">d<") || strings.Contains(c, "\nd\n") || strings.Contains(c, ">d\n") {
305+
foundDetails = true
306+
}
307+
}
308+
firstPart := longSummary[:100]
309+
lastPart := longSummary[len(longSummary)-100:]
310+
foundFirstPart := false
311+
foundLastPart := false
312+
for _, c := range comments {
313+
if strings.Contains(c, firstPart) {
303314
foundFirstPart = true
304315
}
305-
if strings.Contains(c, longSummary[800:]) {
306-
foundSecondPart = true
316+
if strings.Contains(c, lastPart) {
317+
foundLastPart = true
307318
}
308319
}
309-
if !foundSplitWarning || !foundFirstPart || !foundSecondPart {
310-
t.Errorf("Split output did not contain expected parts.\nSplitWarning: %v\nFirstPart: %v\nSecondPart: %v", foundSplitWarning, foundFirstPart, foundSecondPart)
320+
if !foundSplitWarning || !foundFirstPart || !foundLastPart || !foundDetails {
321+
t.Errorf("Split output did not contain expected parts.\nSplitWarning: %v\nFirstPart: %v\nLastPart: %v\nDetails: %v", foundSplitWarning, foundFirstPart, foundLastPart, foundDetails)
311322
}
312323
})
313324

@@ -404,12 +415,12 @@ func TestBuildComment_Deep(t *testing.T) {
404415
comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 500, "prlink")
405416
require.Greater(t, len(comments), 2) // Should create multiple comments
406417
// First comment should have header and start of details
407-
assert.Contains(t, comments[0], "## Kubechecks id Report")
418+
assert.Contains(t, comments[0], "# Kubechecks id Report")
408419
assert.Contains(t, comments[0], "Long details test")
409420
// Check that split warnings are present in middle comments
410421
foundSplitWarnings := 0
411422
for i := 0; i < len(comments)-1; i++ {
412-
assert.Contains(t, comments[i], "## Kubechecks id Report")
423+
assert.Contains(t, comments[i], "# Kubechecks id Report")
413424
if strings.Contains(comments[i], "> **Warning**: Output length greater than maximum allowed comment size. Continued in next comment") {
414425
foundSplitWarnings++
415426
}
@@ -423,7 +434,7 @@ func TestBuildComment_Deep(t *testing.T) {
423434
m := NewMessage("test", 1, 2, fakeVCS)
424435
m.AddNewApp(ctx, "boundary-app")
425436
// Create content that exactly fits the limit
426-
header := "## Kubechecks id Report\n"
437+
header := "# Kubechecks id Report\n"
427438
footer := "\n\n<small> _Done. CommitSHA: sha_ <small>\n"
428439
appHeader := "\n---\n\n<details>\n<summary>\n\n## ArgoCD Application Checks: `boundary-app` :ok:\n</summary>\n\n"
429440
appFooter := "</details>"
@@ -433,25 +444,25 @@ func TestBuildComment_Deep(t *testing.T) {
433444
comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 1000, "prlink")
434445
// Accept either a single comment of length 1000, or multiple comments that together contain all the content
435446
totalLen := 0
436-
for _, c := range comments {
437-
totalLen += len(c)
438-
}
439-
if !(len(comments) == 1 && len(comments[0]) == 1000) && totalLen < 1000 {
440-
t.Errorf("Expected a single comment of length 1000 or multiple comments totaling at least 1000, got %d comments with total length %d", len(comments), totalLen)
441-
}
442-
// Check that the summary and details are present
443-
foundSummary := false
444447
foundDetails := false
448+
firstPart := summary[:100]
449+
lastPart := summary[len(summary)-100:]
450+
foundFirstPart := false
451+
foundLastPart := false
445452
for _, c := range comments {
446-
if strings.Contains(c, summary) {
447-
foundSummary = true
453+
totalLen += len(c)
454+
if strings.Contains(c, firstPart) {
455+
foundFirstPart = true
456+
}
457+
if strings.Contains(c, lastPart) {
458+
foundLastPart = true
448459
}
449460
if strings.Contains(c, "short details") {
450461
foundDetails = true
451462
}
452463
}
453-
if !foundSummary || !foundDetails {
454-
t.Errorf("Expected summary and details to be present in the output")
464+
if !foundFirstPart || !foundLastPart || !foundDetails {
465+
t.Errorf("Expected summary (first and last part) and details to be present in the output")
455466
}
456467
})
457468

@@ -554,7 +565,7 @@ func TestBuildComment_Deep(t *testing.T) {
554565
comments := m.BuildComment(ctx, time.Now(), "sha", "", false, "id", 1, 1, 100, "prlink")
555566
require.Greater(t, len(comments), 1)
556567
// Should have multiple comments due to very small limit
557-
assert.Contains(t, comments[0], "## Kubechecks id Report")
568+
assert.Contains(t, comments[0], "# Kubechecks id Report")
558569
// Check that we have multiple comments and that the content is split appropriately
559570
totalContent := ""
560571
for _, comment := range comments {

pkg/utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,5 @@ func WithErrorLogging(f func() error, msg string) {
3434
}
3535

3636
func GetMessageHeader(identifier string) string {
37-
return fmt.Sprintf("## Kubechecks %s Report\n", identifier)
37+
return fmt.Sprintf("# Kubechecks %s Report\n", identifier)
3838
}

pkg/utils_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,22 @@ func TestGetMessageHeader(t *testing.T) {
1313
{
1414
name: "empty identifier",
1515
identifier: "",
16-
want: "## Kubechecks Report\n",
16+
want: "# Kubechecks Report\n",
1717
},
1818
{
1919
name: "with identifier",
2020
identifier: "test-instance",
21-
want: "## Kubechecks test-instance Report\n",
21+
want: "# Kubechecks test-instance Report\n",
2222
},
2323
{
2424
name: "with special characters",
2525
identifier: "test-instance-123!@#",
26-
want: "## Kubechecks test-instance-123!@# Report\n",
26+
want: "# Kubechecks test-instance-123!@# Report\n",
2727
},
2828
{
2929
name: "with spaces",
3030
identifier: "test instance",
31-
want: "## Kubechecks test instance Report\n",
31+
want: "# Kubechecks test instance Report\n",
3232
},
3333
}
3434

0 commit comments

Comments
 (0)