Skip to content

Commit

Permalink
Add other errors message and shore up inferred GHA titled (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TAGraves
TAGraves authored Mar 14, 2024
1 parent c0b87a4 commit 2fa181b
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 116 deletions.
1 change: 1 addition & 0 deletions .tool-versions
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
golang 1.19.13
10 changes: 10 additions & 0 deletions internal/cli/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,16 @@ func (s Service) RunSuite(ctx context.Context, cfg RunConfig) (finalErr error) {
}
}

if hasDetails && hasQuarantinedFailedTests && otherErrorCount > 0 {
s.Log.Infoln(
fmt.Sprintf(
"\n%v other %v occurred",
otherErrorCount,
pluralize(otherErrorCount, "failure", "failures"),
),
)
}

// Return the original exit code if there was a non-test error
if runErr != nil && otherErrorCount > 0 {
return errors.WithStack(runErr)
Expand Down
235 changes: 120 additions & 115 deletions internal/cli/run_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ var _ = Describe("Run", func() {
})
})

Context("all tests quarantined, but there are other errors", func() {
Context("some quarantined tests successful", func() {
BeforeEach(func() {
mockGetRunConfiguration := func(
ctx context.Context,
Expand All @@ -907,54 +907,24 @@ var _ = Describe("Run", func() {
StrictIdentity: true,
},
},
},
}, nil
}
service.API.(*mocks.API).MockGetRunConfiguration = mockGetRunConfiguration

service.ParseConfig.MutuallyExclusiveParsers[0].(*mocks.Parser).MockParse = func(r io.Reader) (
*v1.TestResults,
error,
) {
return &v1.TestResults{
OtherErrors: []v1.OtherError{
{Message: "uh oh another error"},
},
Tests: []v1.Test{
{
Name: firstSuccessfulTestDescription,
Location: &v1.Location{File: "/path/to/file.test"},
Attempt: v1.TestAttempt{
Status: v1.NewSuccessfulTestStatus(),
},
},
{
Name: firstFailedTestDescription,
Location: &v1.Location{File: "/path/to/file.test"},
Attempt: v1.TestAttempt{
Status: v1.NewFailedTestStatus(nil, nil, nil),
},
},
{
Name: secondFailedTestDescription,
Location: &v1.Location{File: "/other/path/to/file.test"},
Attempt: v1.TestAttempt{
Status: v1.NewFailedTestStatus(nil, nil, nil),
Test: backend.Test{
CompositeIdentifier: fmt.Sprintf("%v -captain- %v", firstSuccessfulTestDescription, "/path/to/file.test"),
IdentityComponents: []string{"description", "file"},
StrictIdentity: true,
},
},
},
}, nil
}
service.API.(*mocks.API).MockGetRunConfiguration = mockGetRunConfiguration
})

It("returns the error code of the command", func() {
Expect(err).To(HaveOccurred())
executionError, ok := errors.AsExecutionError(err)
Expect(ok).To(BeTrue(), "Error is an execution error")
Expect(executionError.Code).To(Equal(exitCode))
It("doesn't return an error", func() {
Expect(err).ToNot(HaveOccurred())
})

It("logs the quarantined tests", func() {
It("reports the original status of success", func() {
logMessages := make([]string, 0)

for _, log := range recordedLogs.All() {
Expand All @@ -967,14 +937,13 @@ var _ = Describe("Run", func() {
Expect(logMessages).To(ContainElement(fmt.Sprintf("- %v", secondFailedTestDescription)))

Expect(uploadedTestResults).ToNot(BeNil())
Expect(uploadedTestResults.Summary.Quarantined).To(Equal(2))
Expect(uploadedTestResults.Tests[0].Attempt.Status.Kind).To(Equal(v1.TestStatusSuccessful))

Expect(uploadedTestResults.Tests[1].Attempt.Status.Kind).To(Equal(v1.TestStatusQuarantined))
Expect(uploadedTestResults.Tests[1].Attempt.Status.OriginalStatus.Kind).To(Equal(v1.TestStatusFailed))

Expect(uploadedTestResults.Tests[2].Attempt.Status.Kind).To(Equal(v1.TestStatusQuarantined))
Expect(uploadedTestResults.Tests[2].Attempt.Status.OriginalStatus.Kind).To(Equal(v1.TestStatusFailed))
Expect(uploadedTestResults.Tests[2].Attempt.Status.OriginalStatus.Kind).To(Equal(v1.TestStatusTimedOut))
})

Context("ABQ", func() {
Expand All @@ -995,99 +964,135 @@ var _ = Describe("Run", func() {
Expect(mockAbqCommandArgs).To(Equal([]string{
"set-exit-code",
"--run-id", "not-a-real-run-id",
"--exit-code", "1",
"--exit-code", "0",
}))
Expect(err).To(HaveOccurred())
Expect(err).NotTo(HaveOccurred())
})
})
})
})
})

Context("some quarantined tests successful", func() {
BeforeEach(func() {
mockGetRunConfiguration := func(
ctx context.Context,
testSuiteIdentifier string,
) (backend.RunConfiguration, error) {
return backend.RunConfiguration{
QuarantinedTests: []backend.QuarantinedTest{
{
Test: backend.Test{
CompositeIdentifier: fmt.Sprintf("%v -captain- %v", firstFailedTestDescription, "/path/to/file.test"),
IdentityComponents: []string{"description", "file"},
StrictIdentity: true,
},
Context("with other errors", func() {
var (
exitCode int
firstFailedTestDescription string
secondFailedTestDescription string
firstSuccessfulTestDescription string
)

BeforeEach(func() {
exitCode = int(GinkgoRandomSeed() + 1)
firstFailedTestDescription = fmt.Sprintf("first-failed-description-%d", GinkgoRandomSeed()+2)
secondFailedTestDescription = fmt.Sprintf("second-failed-description-%d", GinkgoRandomSeed()+3)
firstSuccessfulTestDescription = fmt.Sprintf("first-successful-description-%d", GinkgoRandomSeed()+4)

mockGetExitStatusFromError := func(error) (int, error) {
return exitCode, nil
}
service.TaskRunner.(*mocks.TaskRunner).MockGetExitStatusFromError = mockGetExitStatusFromError

service.ParseConfig.MutuallyExclusiveParsers[0].(*mocks.Parser).MockParse = func(r io.Reader) (
*v1.TestResults,
error,
) {
return &v1.TestResults{
OtherErrors: []v1.OtherError{
{
Message: "Something went wrong",
},
},
Tests: []v1.Test{
{
Name: firstSuccessfulTestDescription,
Location: &v1.Location{File: "/path/to/file.test"},
Attempt: v1.TestAttempt{
Status: v1.NewSuccessfulTestStatus(),
},
{
Test: backend.Test{
CompositeIdentifier: fmt.Sprintf("%v -captain- %v", secondFailedTestDescription, "/other/path/to/file.test"),
IdentityComponents: []string{"description", "file"},
StrictIdentity: true,
},
},
{
Name: firstFailedTestDescription,
Location: &v1.Location{File: "/path/to/file.test"},
Attempt: v1.TestAttempt{
Status: v1.NewFailedTestStatus(nil, nil, nil),
},
{
Test: backend.Test{
CompositeIdentifier: fmt.Sprintf("%v -captain- %v", firstSuccessfulTestDescription, "/path/to/file.test"),
IdentityComponents: []string{"description", "file"},
StrictIdentity: true,
},
},
{
Name: secondFailedTestDescription,
Location: &v1.Location{File: "/other/path/to/file.test"},
Attempt: v1.TestAttempt{
Status: v1.NewTimedOutTestStatus(),
},
},
}, nil
}
service.API.(*mocks.API).MockGetRunConfiguration = mockGetRunConfiguration
})
},
}, nil
}

It("doesn't return an error", func() {
Expect(err).ToNot(HaveOccurred())
})
mockUploadTestResults := func(
ctx context.Context,
testSuite string,
testResults v1.TestResults,
) ([]backend.TestResultsUploadResult, error) {
testResultsFileUploaded = true

It("reports the original status of success", func() {
logMessages := make([]string, 0)
return []backend.TestResultsUploadResult{
{OriginalPaths: []string{testResultsFilePath}, Uploaded: true},
{OriginalPaths: []string{"./fake/path/1.json", "./fake/path/2.json"}, Uploaded: false},
}, nil
}
service.API.(*mocks.API).MockUpdateTestResults = mockUploadTestResults

for _, log := range recordedLogs.All() {
logMessages = append(logMessages, log.Message)
}
mockGetRunConfiguration := func(
ctx context.Context,
testSuiteIdentifier string,
) (backend.RunConfiguration, error) {
return backend.RunConfiguration{
QuarantinedTests: []backend.QuarantinedTest{
{
Test: backend.Test{
CompositeIdentifier: fmt.Sprintf("%v -captain- %v", firstFailedTestDescription, "/path/to/file.test"),
IdentityComponents: []string{"description", "file"},
StrictIdentity: true,
},
},
{
Test: backend.Test{
CompositeIdentifier: fmt.Sprintf("%v -captain- %v", secondFailedTestDescription, "/other/path/to/file.test"),
IdentityComponents: []string{"description", "file"},
StrictIdentity: true,
},
},
},
}, nil
}
service.API.(*mocks.API).MockGetRunConfiguration = mockGetRunConfiguration
})

Expect(logMessages).To(ContainElement(ContainSubstring("2 of 2 failures under quarantine")))
Expect(logMessages).NotTo(ContainElement(ContainSubstring("remaining actionable")))
Expect(logMessages).To(ContainElement(fmt.Sprintf("- %v", firstFailedTestDescription)))
Expect(logMessages).To(ContainElement(fmt.Sprintf("- %v", secondFailedTestDescription)))
It("returns the error code of the command", func() {
Expect(err).To(HaveOccurred())
executionError, ok := errors.AsExecutionError(err)
Expect(ok).To(BeTrue(), "Error is an execution error")
Expect(executionError.Code).To(Equal(exitCode))
})

Expect(uploadedTestResults).ToNot(BeNil())
Expect(uploadedTestResults.Tests[0].Attempt.Status.Kind).To(Equal(v1.TestStatusSuccessful))
It("logs the quarantined tests", func() {
logMessages := make([]string, 0)

Expect(uploadedTestResults.Tests[1].Attempt.Status.Kind).To(Equal(v1.TestStatusQuarantined))
Expect(uploadedTestResults.Tests[1].Attempt.Status.OriginalStatus.Kind).To(Equal(v1.TestStatusFailed))
for _, log := range recordedLogs.All() {
logMessages = append(logMessages, log.Message)
}

Expect(uploadedTestResults.Tests[2].Attempt.Status.Kind).To(Equal(v1.TestStatusQuarantined))
Expect(uploadedTestResults.Tests[2].Attempt.Status.OriginalStatus.Kind).To(Equal(v1.TestStatusTimedOut))
})
Expect(logMessages).To(ContainElement(ContainSubstring("2 of 2 failures under quarantine")))
})

Context("ABQ", func() {
Context("with a supervisor ABQ state file", func() {
BeforeEach(func() {
abqStateFileExists = true
abqStateJSON = fmt.Sprintf(`{
"abq_version": "1.1.2",
"abq_executable": "%s",
"run_id": "not-a-real-run-id",
"supervisor": true
}`, abqExecutable)
})
It("logs the other error count", func() {
logMessages := make([]string, 0)

It("calls abq set-exit-code", func() {
Expect(abqCommandStarted).To(BeTrue())
Expect(abqCommandFinished).To(BeTrue())
Expect(mockAbqCommandArgs).To(Equal([]string{
"set-exit-code",
"--run-id", "not-a-real-run-id",
"--exit-code", "0",
}))
Expect(err).NotTo(HaveOccurred())
})
})
})
for _, log := range recordedLogs.All() {
logMessages = append(logMessages, log.Message)
}

Expect(logMessages).To(ContainElement(ContainSubstring("1 other failure occurred")))
})
})

Expand Down
18 changes: 17 additions & 1 deletion internal/providers/github_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ type GitHubEnv struct {
// commit sha
CommitSha string `env:"GITHUB_SHA"`

// workflow
Workflow string `env:"GITHUB_WORKFLOW"`

// tags
Attempt string `env:"GITHUB_RUN_ATTEMPT"`
ID string `env:"GITHUB_RUN_ID"`
Expand All @@ -43,6 +46,10 @@ type GitHubEventPayloadData struct {
Number int `json:"number"`
Title string `json:"title"`
} `json:"pull_request"`
Issue struct {
Number int `json:"number"`
Title string `json:"title"`
} `json:"issue"`
}

func (cfg GitHubEnv) makeProvider() (Provider, error) {
Expand Down Expand Up @@ -77,10 +84,19 @@ func (cfg GitHubEnv) MakeProviderWithoutCommitMessageParsing(payloadData GitHubE
return Provider{}, validationError
}

if payloadData.PullRequest.Title == "" && payloadData.Issue.Title != "" {
payloadData.PullRequest.Title = payloadData.Issue.Title
payloadData.PullRequest.Number = payloadData.Issue.Number
}

commitMessage := payloadData.HeadCommit.Message
var title string
if commitMessage == "" {
title = fmt.Sprintf("%s (PR #%d)", payloadData.PullRequest.Title, payloadData.PullRequest.Number)
if payloadData.PullRequest.Title == "" {
title = cfg.Workflow
} else {
title = fmt.Sprintf("%s (PR #%d)", payloadData.PullRequest.Title, payloadData.PullRequest.Number)
}
}

provider := Provider{
Expand Down
Loading

0 comments on commit 2fa181b

Please sign in to comment.