diff --git a/packages/kbn-test/src/failed_tests_reporter/existing_failed_test_issues.test.ts b/packages/kbn-test/src/failed_tests_reporter/existing_failed_test_issues.test.ts new file mode 100644 index 0000000000000..7c28f105fba75 --- /dev/null +++ b/packages/kbn-test/src/failed_tests_reporter/existing_failed_test_issues.test.ts @@ -0,0 +1,163 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { ToolingLog, ToolingLogCollectingWriter, createStripAnsiSerializer } from '@kbn/dev-utils'; + +import type { TestFailure } from './get_failures'; +import { ExistingFailedTestIssues, FailedTestIssue } from './existing_failed_test_issues'; + +expect.addSnapshotSerializer(createStripAnsiSerializer()); + +const log = new ToolingLog(); +const writer = new ToolingLogCollectingWriter(); +log.setWriters([writer]); + +afterEach(() => { + writer.messages.length = 0; + jest.clearAllMocks(); +}); + +jest.mock('axios', () => ({ + request: jest.fn(), +})); +const Axios = jest.requireMock('axios'); + +const mockTestFailure: Omit = { + failure: '', + likelyIrrelevant: false, + time: '100', + 'metadata-json': '', + 'system-out': '', +}; + +it('captures a list of failed test issue, loads the bodies for each issue, and only fetches what is needed', async () => { + const existing = new ExistingFailedTestIssues(log); + + Axios.request.mockImplementation(({ data }: any) => ({ + data: { + existingIssues: data.failures + .filter((t: any) => t.classname.includes('foo')) + .map( + (t: any, i: any): FailedTestIssue => ({ + classname: t.classname, + name: t.name, + github: { + htmlUrl: `htmlurl(${t.classname}/${t.name})`, + nodeId: `nodeid(${t.classname}/${t.name})`, + number: (i + 1) * (t.classname.length + t.name.length), + body: `FAILURE: ${t.classname}/${t.name}`, + }, + }) + ), + }, + })); + + const fooFailure: TestFailure = { + ...mockTestFailure, + classname: 'foo classname', + name: 'foo test', + }; + const barFailure: TestFailure = { + ...mockTestFailure, + classname: 'bar classname', + name: 'bar test', + }; + + await existing.loadForFailures([fooFailure]); + await existing.loadForFailures([fooFailure, barFailure]); + + expect(existing.getForFailure(fooFailure)).toMatchInlineSnapshot(` + Object { + "classname": "foo classname", + "github": Object { + "body": "FAILURE: foo classname/foo test", + "htmlUrl": "htmlurl(foo classname/foo test)", + "nodeId": "nodeid(foo classname/foo test)", + "number": 21, + }, + "name": "foo test", + } + `); + expect(existing.getForFailure(barFailure)).toMatchInlineSnapshot(`undefined`); + + expect(writer.messages).toMatchInlineSnapshot(` + Array [ + " debg finding 1 existing issues via ci-stats", + " debg found 1 existing issues", + " debg loaded 1 existing test issues", + " debg finding 1 existing issues via ci-stats", + " debg found 0 existing issues", + " debg loaded 1 existing test issues", + ] + `); + expect(Axios.request).toMatchInlineSnapshot(` + [MockFunction] { + "calls": Array [ + Array [ + Object { + "baseURL": "https://ci-stats.kibana.dev", + "data": Object { + "failures": Array [ + Object { + "classname": "foo classname", + "name": "foo test", + }, + ], + }, + "method": "POST", + "url": "/v1/find_failed_test_issues", + }, + ], + Array [ + Object { + "baseURL": "https://ci-stats.kibana.dev", + "data": Object { + "failures": Array [ + Object { + "classname": "bar classname", + "name": "bar test", + }, + ], + }, + "method": "POST", + "url": "/v1/find_failed_test_issues", + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": Object { + "data": Object { + "existingIssues": Array [ + Object { + "classname": "foo classname", + "github": Object { + "body": "FAILURE: foo classname/foo test", + "htmlUrl": "htmlurl(foo classname/foo test)", + "nodeId": "nodeid(foo classname/foo test)", + "number": 21, + }, + "name": "foo test", + }, + ], + }, + }, + }, + Object { + "type": "return", + "value": Object { + "data": Object { + "existingIssues": Array [], + }, + }, + }, + ], + } + `); +}); diff --git a/packages/kbn-test/src/failed_tests_reporter/existing_failed_test_issues.ts b/packages/kbn-test/src/failed_tests_reporter/existing_failed_test_issues.ts new file mode 100644 index 0000000000000..9a38cb0c659d3 --- /dev/null +++ b/packages/kbn-test/src/failed_tests_reporter/existing_failed_test_issues.ts @@ -0,0 +1,156 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { setTimeout } from 'timers/promises'; + +import Axios from 'axios'; +import { ToolingLog, isAxiosRequestError, isAxiosResponseError } from '@kbn/dev-utils'; + +import { GithubIssueMini } from './github_api'; +import { TestFailure } from './get_failures'; + +export interface FailedTestIssue { + classname: string; + name: string; + github: { + nodeId: string; + number: number; + htmlUrl: string; + body: string; + }; +} + +interface FindFailedTestIssuesResponse { + existingIssues: FailedTestIssue[]; +} + +export interface ExistingFailedTestIssue extends FailedTestIssue { + github: FailedTestIssue['github'] & { + body: string; + }; +} + +const BASE_URL = 'https://ci-stats.kibana.dev'; + +/** + * In order to deal with rate limits imposed on our Github API tokens we needed + * to stop iterating through all the Github issues to find previously created issues + * for a test failure. This class uses the ci-stats API to lookup the mapping between + * failed tests and the existing failed-tests issues. The API maintains an index of + * this mapping in ES to make much better use of the Github API. + */ +export class ExistingFailedTestIssues { + private readonly results = new Map(); + + constructor(private readonly log: ToolingLog) {} + + async loadForFailures(newFailures: TestFailure[]) { + const unseenFailures: TestFailure[] = []; + for (const failure of newFailures) { + if (!this.isFailureSeen(failure)) { + unseenFailures.push(failure); + } + } + + if (unseenFailures.length === 0) { + this.log.debug('no unseen issues in new batch of failures'); + return; + } + + this.log.debug('finding', unseenFailures.length, 'existing issues via ci-stats'); + const failedTestIssues = await this.findExistingIssues(unseenFailures); + this.log.debug('found', failedTestIssues.length, 'existing issues'); + + const initialResultSize = this.results.size; + for (const failure of unseenFailures) { + const ciStatsIssue = failedTestIssues.find( + (i) => i.classname === failure.classname && i.name === failure.name + ); + if (!ciStatsIssue) { + this.results.set(failure, undefined); + continue; + } + + this.results.set(failure, ciStatsIssue); + } + + this.log.debug('loaded', this.results.size - initialResultSize, 'existing test issues'); + } + + getForFailure(failure: TestFailure) { + for (const [f, issue] of this.results) { + if (f.classname === failure.classname && f.name === failure.name) { + return issue; + } + } + } + + addNewlyCreated(failure: TestFailure, newIssue: GithubIssueMini) { + this.results.set(failure, { + classname: failure.classname, + name: failure.name, + github: { + body: newIssue.body, + htmlUrl: newIssue.html_url, + nodeId: newIssue.node_id, + number: newIssue.number, + }, + }); + } + + private async findExistingIssues(failures: TestFailure[]) { + if (failures.length === 0) { + return []; + } + + const maxAttempts = 5; + let attempt = 0; + while (true) { + attempt += 1; + + try { + const resp = await Axios.request({ + method: 'POST', + baseURL: BASE_URL, + url: '/v1/find_failed_test_issues', + data: { + failures: failures.map((f) => ({ + classname: f.classname, + name: f.name, + })), + }, + }); + + return resp.data.existingIssues; + } catch (error: unknown) { + if ( + attempt < maxAttempts && + ((isAxiosResponseError(error) && error.response.status >= 500) || + isAxiosRequestError(error)) + ) { + this.log.error(error); + this.log.warning(`Failure talking to ci-stats, waiting ${attempt} before retrying`); + await setTimeout(attempt * 1000); + continue; + } + + throw error; + } + } + } + + private isFailureSeen(failure: TestFailure) { + for (const seen of this.results.keys()) { + if (seen.classname === failure.classname && seen.name === failure.name) { + return true; + } + } + + return false; + } +} diff --git a/packages/kbn-test/src/failed_tests_reporter/github_api.ts b/packages/kbn-test/src/failed_tests_reporter/github_api.ts index bb7570225a013..bf870045c5a6e 100644 --- a/packages/kbn-test/src/failed_tests_reporter/github_api.ts +++ b/packages/kbn-test/src/failed_tests_reporter/github_api.ts @@ -9,7 +9,6 @@ import Url from 'url'; import Axios, { AxiosRequestConfig, AxiosInstance } from 'axios'; -import parseLinkHeader from 'parse-link-header'; import { ToolingLog, isAxiosResponseError, isAxiosRequestError } from '@kbn/dev-utils'; const BASE_URL = 'https://api.github.com/repos/elastic/kibana/'; @@ -17,6 +16,7 @@ const BASE_URL = 'https://api.github.com/repos/elastic/kibana/'; export interface GithubIssue { html_url: string; number: number; + node_id: string; title: string; labels: unknown[]; body: string; @@ -29,6 +29,7 @@ export interface GithubIssueMini { number: GithubIssue['number']; body: GithubIssue['body']; html_url: GithubIssue['html_url']; + node_id: GithubIssue['node_id']; } type RequestOptions = AxiosRequestConfig & { @@ -73,70 +74,6 @@ export class GithubApi { return this.requestCount; } - private failedTestIssuesPageCache: { - pages: GithubIssue[][]; - nextRequest: RequestOptions | undefined; - } = { - pages: [], - nextRequest: { - safeForDryRun: true, - method: 'GET', - url: Url.resolve(BASE_URL, 'issues'), - params: { - state: 'all', - per_page: '100', - labels: 'failed-test', - sort: 'updated', - direction: 'desc', - }, - }, - }; - - /** - * Iterate the `failed-test` issues from elastic/kibana, each response - * from Github is cached and subsequent calls to this method will first - * iterate the previous responses from Github, then start requesting - * more pages of issues from github until all pages have been cached. - * - * Aborting the iterator part way through will prevent unnecessary request - * to Github from being issued. - */ - async *iterateCachedFailedTestIssues() { - const cache = this.failedTestIssuesPageCache; - - // start from page 0, and progress forward if we have cache or a request that will load that cache page - for (let page = 0; page < cache.pages.length || cache.nextRequest; page++) { - if (page >= cache.pages.length && cache.nextRequest) { - const resp = await this.request(cache.nextRequest, []); - cache.pages.push(resp.data); - - const link = - typeof resp.headers.link === 'string' ? parseLinkHeader(resp.headers.link) : undefined; - - cache.nextRequest = - link && link.next && link.next.url - ? { - safeForDryRun: true, - method: 'GET', - url: link.next.url, - } - : undefined; - } - - for (const issue of cache.pages[page]) { - yield issue; - } - } - } - - async findFailedTestIssue(test: (issue: GithubIssue) => boolean) { - for await (const issue of this.iterateCachedFailedTestIssues()) { - if (test(issue)) { - return issue; - } - } - } - async editIssueBodyAndEnsureOpen(issueNumber: number, newBody: string) { await this.request( { @@ -179,6 +116,7 @@ export class GithubApi { body, number: 999, html_url: 'https://dryrun', + node_id: 'adflksdjf', } ); diff --git a/packages/kbn-test/src/failed_tests_reporter/report_failure.test.ts b/packages/kbn-test/src/failed_tests_reporter/report_failure.test.ts index b2fd3de6bbbbc..fd445888ad1ee 100644 --- a/packages/kbn-test/src/failed_tests_reporter/report_failure.test.ts +++ b/packages/kbn-test/src/failed_tests_reporter/report_failure.test.ts @@ -67,13 +67,18 @@ describe('updateFailureIssue()', () => { await updateFailureIssue( 'https://build-url', { - html_url: 'https://github.com/issues/1234', - number: 1234, - body: dedent` - # existing issue body + classname: 'foo', + name: 'test', + github: { + htmlUrl: 'https://github.com/issues/1234', + number: 1234, + nodeId: 'abcd', + body: dedent` + # existing issue body - " - `, + " + `, + }, }, api, 'main' diff --git a/packages/kbn-test/src/failed_tests_reporter/report_failure.ts b/packages/kbn-test/src/failed_tests_reporter/report_failure.ts index c44fae560156a..e2750f15b8720 100644 --- a/packages/kbn-test/src/failed_tests_reporter/report_failure.ts +++ b/packages/kbn-test/src/failed_tests_reporter/report_failure.ts @@ -7,8 +7,9 @@ */ import { TestFailure } from './get_failures'; -import { GithubIssueMini, GithubApi } from './github_api'; +import { GithubApi } from './github_api'; import { getIssueMetadata, updateIssueMetadata } from './issue_metadata'; +import { ExistingFailedTestIssue } from './existing_failed_test_issues'; export async function createFailureIssue( buildUrl: string, @@ -40,18 +41,21 @@ export async function createFailureIssue( export async function updateFailureIssue( buildUrl: string, - issue: GithubIssueMini, + issue: ExistingFailedTestIssue, api: GithubApi, branch: string ) { // Increment failCount - const newCount = getIssueMetadata(issue.body, 'test.failCount', 0) + 1; - const newBody = updateIssueMetadata(issue.body, { + const newCount = getIssueMetadata(issue.github.body, 'test.failCount', 0) + 1; + const newBody = updateIssueMetadata(issue.github.body, { 'test.failCount': newCount, }); - await api.editIssueBodyAndEnsureOpen(issue.number, newBody); - await api.addIssueComment(issue.number, `New failure: [CI Build - ${branch}](${buildUrl})`); + await api.editIssueBodyAndEnsureOpen(issue.github.number, newBody); + await api.addIssueComment( + issue.github.number, + `New failure: [CI Build - ${branch}](${buildUrl})` + ); - return newCount; + return { newBody, newCount }; } diff --git a/packages/kbn-test/src/failed_tests_reporter/run_failed_tests_reporter_cli.ts b/packages/kbn-test/src/failed_tests_reporter/run_failed_tests_reporter_cli.ts index 6ab135a6afa7e..1039c20cfe853 100644 --- a/packages/kbn-test/src/failed_tests_reporter/run_failed_tests_reporter_cli.ts +++ b/packages/kbn-test/src/failed_tests_reporter/run_failed_tests_reporter_cli.ts @@ -13,16 +13,16 @@ import { run, createFailError, createFlagError, CiStatsReporter } from '@kbn/dev import globby from 'globby'; import normalize from 'normalize-path'; -import { getFailures, TestFailure } from './get_failures'; -import { GithubApi, GithubIssueMini } from './github_api'; +import { getFailures } from './get_failures'; +import { GithubApi } from './github_api'; import { updateFailureIssue, createFailureIssue } from './report_failure'; -import { getIssueMetadata } from './issue_metadata'; import { readTestReport } from './test_report'; import { addMessagesToReport } from './add_messages_to_report'; import { getReportMessageIter } from './report_metadata'; import { reportFailuresToEs } from './report_failures_to_es'; import { reportFailuresToFile } from './report_failures_to_file'; import { getBuildkiteMetadata } from './buildkite_metadata'; +import { ExistingFailedTestIssues } from './existing_failed_test_issues'; const DEFAULT_PATTERNS = [Path.resolve(REPO_ROOT, 'target/junit/**/*.xml')]; @@ -93,15 +93,14 @@ export function runFailedTestsReporterCli() { } log.info('found', reportPaths.length, 'junit reports', reportPaths); - const newlyCreatedIssues: Array<{ - failure: TestFailure; - newIssue: GithubIssueMini; - }> = []; + const existingIssues = new ExistingFailedTestIssues(log); for (const reportPath of reportPaths) { const report = await readTestReport(reportPath); const messages = Array.from(getReportMessageIter(report)); - const failures = await getFailures(report); + const failures = getFailures(report); + + await existingIssues.loadForFailures(failures); if (indexInEs) { await reportFailuresToEs(log, failures); @@ -124,50 +123,32 @@ export function runFailedTestsReporterCli() { continue; } - let existingIssue: GithubIssueMini | undefined = updateGithub - ? await githubApi.findFailedTestIssue( - (i) => - getIssueMetadata(i.body, 'test.class') === failure.classname && - getIssueMetadata(i.body, 'test.name') === failure.name - ) - : undefined; - - if (!existingIssue) { - const newlyCreated = newlyCreatedIssues.find( - ({ failure: f }) => f.classname === failure.classname && f.name === failure.name - ); - - if (newlyCreated) { - existingIssue = newlyCreated.newIssue; - } - } - + const existingIssue = existingIssues.getForFailure(failure); if (existingIssue) { - const newFailureCount = await updateFailureIssue( + const { newBody, newCount } = await updateFailureIssue( buildUrl, existingIssue, githubApi, branch ); - const url = existingIssue.html_url; + const url = existingIssue.github.htmlUrl; + existingIssue.github.body = newBody; failure.githubIssue = url; - failure.failureCount = updateGithub ? newFailureCount : newFailureCount - 1; - pushMessage( - `Test has failed ${newFailureCount - 1} times on tracked branches: ${url}` - ); + failure.failureCount = updateGithub ? newCount : newCount - 1; + pushMessage(`Test has failed ${newCount - 1} times on tracked branches: ${url}`); if (updateGithub) { - pushMessage(`Updated existing issue: ${url} (fail count: ${newFailureCount})`); + pushMessage(`Updated existing issue: ${url} (fail count: ${newCount})`); } continue; } const newIssue = await createFailureIssue(buildUrl, failure, githubApi, branch); + existingIssues.addNewlyCreated(failure, newIssue); pushMessage('Test has not failed recently on tracked branches'); if (updateGithub) { pushMessage(`Created new issue: ${newIssue.html_url}`); failure.githubIssue = newIssue.html_url; } - newlyCreatedIssues.push({ failure, newIssue }); failure.failureCount = updateGithub ? 1 : 0; }