Skip to content

Commit

Permalink
Merge #30803
Browse files Browse the repository at this point in the history
30803: roachtest: don't include "parent" tests in slack summary r=benesch a=petermattis

This mirrors the behavior for issue posting which only posts about
failures for tests with a Run function.

Rework how skipped tests are reported. The previous reporting of the
number of skipped tests has been broken since the introduction of
subtests. Now we also report the names of the skipped tests as a
reminder that they are skipped.

Fixes #30800

Release note: None

Co-authored-by: Peter Mattis <[email protected]>
  • Loading branch information
craig[bot] and petermattis committed Sep 30, 2018
2 parents f40101b + 5526767 commit 6692789
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 7 deletions.
9 changes: 7 additions & 2 deletions pkg/cmd/roachtest/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@ func sortTests(tests []*test) {
})
}

func postSlackReport(pass, fail map[*test]struct{}, skip int) {
func postSlackReport(pass, fail, skip map[*test]struct{}) {
var stablePass []*test
var stableFail []*test
var unstablePass []*test
var unstableFail []*test
var skipped []*test
for t := range pass {
if t.spec.Stable {
stablePass = append(stablePass, t)
Expand All @@ -73,6 +74,9 @@ func postSlackReport(pass, fail map[*test]struct{}, skip int) {
unstableFail = append(unstableFail, t)
}
}
for t := range skip {
skipped = append(skipped, t)
}

client := makeSlackClient()
if client == nil {
Expand All @@ -93,7 +97,7 @@ func postSlackReport(pass, fail map[*test]struct{}, skip int) {
branch = b
}
message := fmt.Sprintf("%s: %d passed, %d failed, %d skipped",
branch, len(stablePass), len(stableFail), skip)
branch, len(stablePass), len(stableFail), len(skipped))

{
status := "good"
Expand Down Expand Up @@ -123,6 +127,7 @@ func postSlackReport(pass, fail map[*test]struct{}, skip int) {
{stableFail, "Failures", "danger"},
{unstablePass, "Successes [unstable]", "good"},
{unstableFail, "Failures [unstable]", "warning"},
{skipped, "Skipped", "warning"},
}
for _, d := range data {
if len(d.tests) > 0 {
Expand Down
17 changes: 12 additions & 5 deletions pkg/cmd/roachtest/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ type registry struct {
running map[*test]struct{}
pass map[*test]struct{}
fail map[*test]struct{}
skip map[*test]struct{}
}
}

Expand Down Expand Up @@ -266,6 +267,7 @@ func (r *registry) Run(filter []string) int {
r.status.running = make(map[*test]struct{})
r.status.pass = make(map[*test]struct{})
r.status.fail = make(map[*test]struct{})
r.status.skip = make(map[*test]struct{})

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down Expand Up @@ -305,7 +307,7 @@ func (r *registry) Run(filter []string) int {
case <-done:
r.status.Lock()
defer r.status.Unlock()
postSlackReport(r.status.pass, r.status.fail, len(r.m)-len(tests))
postSlackReport(r.status.pass, r.status.fail, r.status.skip)

stableFails := 0
for t := range r.status.fail {
Expand Down Expand Up @@ -712,10 +714,15 @@ func (r *registry) run(

r.status.Lock()
delete(r.status.running, t)
if t.Failed() {
r.status.fail[t] = struct{}{}
} else {
r.status.pass[t] = struct{}{}
// Only include tests with a Run function in the summary output.
if t.spec.Run != nil {
if t.Failed() {
r.status.fail[t] = struct{}{}
} else if t.spec.Skip == "" {
r.status.pass[t] = struct{}{}
} else {
r.status.skip[t] = struct{}{}
}
}
r.status.Unlock()

Expand Down

0 comments on commit 6692789

Please sign in to comment.