Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output logs when failures happen but not in a test #104

Merged
merged 3 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/go-test-results-parsing-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
id: action_run
continue-on-error: true
with:
results-file: ./go/go-test-results-parsing/__tests__/__fixtures__/go_test_results_input.json
results-file: ./go/go-test-results-parsing/__tests__/fixtures/go_test_results_input.json
output-mode: ${{ matrix.output-mode }}
- name: Check for expected failure
run: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ exports[`output testing failed test list 1`] = `
]
`;

exports[`output testing failure without a test failure 1`] = `[]`;

exports[`output testing json test output 1`] = `
[
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{"Time":"2023-08-03T15:42:15.857924669Z","Action":"output","Package":"github.com/smartcontractkit/chainlink/v2/core/gethwrappers","Test":"TestCheckContractHashesFromLastGoGenerate","Output":"=== RUN TestCheckContractHashesFromLastGoGenerate\n"}
{"Time":"2023-08-03T15:42:15.867984853Z","Action":"output","Package":"github.com/smartcontractkit/chainlink/v2/core/gethwrappers","Test":"TestCheckContractHashesFromLastGoGenerate","Output":"Could not read abi path to create version hash: open ../../contracts/solc/v0.8.6/FunctionsBillingRegistryEventsMock.abi: no such file or directory\n"}
{"Time":"2023-08-03T15:42:15.869135019Z","Action":"output","Package":"github.com/smartcontractkit/chainlink/v2/core/gethwrappers","Output":"FAIL\tgithub.com/smartcontractkit/chainlink/v2/core/gethwrappers\t74.285s\n"}
{"Time":"2023-08-03T15:42:15.86915082Z","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/gethwrappers","Elapsed":74.285}
36 changes: 27 additions & 9 deletions go/go-test-results-parsing/__tests__/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import {
deserializeTestResultsFile,
jsonTestOutput,
getTestFailures,
standardTestOutput
standardTestOutput,
logAllOutput
} from '../src/main'
import {TestResult} from '../src/types'
import {TestResult, TestRunFailures} from '../src/types'
const file = './__tests__/fixtures/go_test_results_input.json'
describe('output testing', () => {
test('can deserialize file', () => {
Expand All @@ -14,30 +15,47 @@ describe('output testing', () => {
})
test('failed test list', () => {
const testResults: TestResult[] = deserializeTestResultsFile(file)
const failedTests: string[] = getTestFailures(testResults)
expect(failedTests).toMatchSnapshot()
expect(failedTests.length).toBe(1)
const failedTests: TestRunFailures = getTestFailures(testResults)
expect(failedTests.TestsFailed).toMatchSnapshot()
expect(failedTests.TestsFailed.length).toBe(1)
expect(failedTests.PackageFailure).toBe(true)
})
test('standard test output', () => {
const testResults: TestResult[] = deserializeTestResultsFile(file)
const failedTests: string[] = getTestFailures(testResults)
const failedTests: TestRunFailures = getTestFailures(testResults)
const filteredResults: TestResult[] = standardTestOutput(
failedTests,
failedTests.TestsFailed,
testResults,
true
)
expect(filteredResults).toMatchSnapshot()
expect(filteredResults.length).toBe(1)
expect(failedTests.PackageFailure).toBe(true)
})
test('json test output', () => {
const testResults: TestResult[] = deserializeTestResultsFile(file)
const failedTests: string[] = getTestFailures(testResults)
const failedTests: TestRunFailures = getTestFailures(testResults)
const filteredResults: TestResult[] = jsonTestOutput(
failedTests,
failedTests.TestsFailed,
testResults,
true
)
expect(filteredResults).toMatchSnapshot()
expect(filteredResults.length).toBe(5)
expect(failedTests.PackageFailure).toBe(true)
})
test('failure without a test failure', () => {
const file = './__tests__/fixtures/failure_output_without_test_failure.json'
const testResults: TestResult[] = deserializeTestResultsFile(file)
const failedTests: TestRunFailures = getTestFailures(testResults)
const filteredResults: TestResult[] = standardTestOutput(
failedTests.TestsFailed,
testResults,
true
)
expect(filteredResults).toMatchSnapshot()
expect(filteredResults.length).toBe(0)
expect(failedTests.PackageFailure).toBe(true)
logAllOutput(testResults)
})
})
44 changes: 39 additions & 5 deletions go/go-test-results-parsing/dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/go-test-results-parsing/dist/index.js.map

Large diffs are not rendered by default.

51 changes: 44 additions & 7 deletions go/go-test-results-parsing/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import * as core from '@actions/core'
import * as fs from 'fs'
import {KebabCasedProperties} from 'type-fest'
import {OutputMode, TestResult, TestResultsSchema} from './types'
import {
OutputMode,
TestResult,
TestResultsSchema,
TestRunFailures
} from './types'

/**
* Github action to parse the output of go test -json output to wanted formats with wanted data
Expand All @@ -15,13 +20,20 @@ async function main(): Promise<void> {
}
const outputMode: OutputMode = getTypedInput('output-mode') as OutputMode
const testResults: TestResult[] = deserializeTestResultsFile(resultsFile)
const failedTests: string[] = getTestFailures(testResults)
const failedTests: TestRunFailures = getTestFailures(testResults)
if (outputMode === 'standard') {
standardTestOutput(failedTests, testResults, true)
standardTestOutput(failedTests.TestsFailed, testResults, true)
} else if (outputMode === 'json') {
jsonTestOutput(failedTests, testResults, true)
jsonTestOutput(failedTests.TestsFailed, testResults, true)
}
if (failedTests.length > 0) {
if (failedTests.TestsFailed.length > 0) {
throw new Error('Test Failures Found')
} else if (
failedTests.TestsFailed.length === 0 &&
failedTests.PackageFailure
) {
// we have a failure without a test failure, log out all the output
logAllOutput(testResults)
throw new Error('Test Failures Found')
}
} catch (error) {
Expand Down Expand Up @@ -49,16 +61,22 @@ export function deserializeTestResultsFile(resultsFile: string): TestResult[] {
* @param testResults The test results to filter through
* @returns The failed test names
*/
export function getTestFailures(testResults: TestResult[]): string[] {
export function getTestFailures(testResults: TestResult[]): TestRunFailures {
const names: string[] = []
let packageFailure = false
testResults.filter(testResult => {
if (testResult.Action === 'fail') {
if (testResult.Test) {
names.push(testResult.Test)
} else {
// it is possible to have a package failure without a test failure
// so we are storing whether we have a package failure to determine
// if we need to output all the output for triage later for panic cases
packageFailure = true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tateexon I'm wondering if this case is expected and if we'll always fall through to this case?

When I was reviewing the output for the flakey test runner I put together, I had to handle both cases because:

  • if a test fails, you'll always get output saying the package failed
  • a package could fail without a test failing (eg. in the case of the test spinning off a goroutine and panicing -- go can't trace the goroutine back to its originating test so just fails the package IIRC).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my comment is the problem here, will update the comment to make more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment in code has been updated to make more sense.

}
}
})
return names
return {TestsFailed: names, PackageFailure: packageFailure} as TestRunFailures
}

/**
Expand Down Expand Up @@ -145,6 +163,25 @@ export function outputHandler(output: string, shouldLog: boolean): void {
}
}

/**
* We have edge cases where we have a failure but no test failure
* In this case we want to output all the output for triage since we
* don't necessarily know what will be useful
* @param testResults The test results to print outputs from
*/
export function logAllOutput(testResults: TestResult[]): void {
let outputString = ''
for (const testResult of testResults) {
if (testResult.Action === 'output') {
outputString += testResult.Output
}
}
core.info(
'We had an error in the test run but no specific test had a failure log, logging out everything for triage'
)
core.info(outputString)
}

/**
* Takes kebob cased inputs and enforces they match the expected inputs
* @param inputKey The input key to get
Expand Down
6 changes: 6 additions & 0 deletions go/go-test-results-parsing/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,9 @@ export const TestResultsSchema = z.array(TestResultSchema)

const outputMode = z.enum(['standard', 'json'])
export type OutputMode = z.infer<typeof outputMode>

const testRunFailures = z.object({
TestsFailed: z.array(z.string()),
PackageFailure: z.boolean()
})
export type TestRunFailures = z.infer<typeof testRunFailures>
Loading