[ci/reportFailures] --dry-run is overloaded, split it up (elastic#52314)

* [ci/reportFailures] --dry-run is overloaded, split it up * force some failures to verify the fix * Revert "force some failures to verify the fix" This reverts commit cf2a58e. * update readme to mention new flags * remove unnecessary commas (cherry picked from commit 8e8571b)
spalger · Dec 5, 2019 · ac14dcd · ac14dcd
1 parent 491b3d4
commit ac14dcd
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 38 deletions.
diff --git a/packages/kbn-test/src/failed_tests_reporter/README.md b/packages/kbn-test/src/failed_tests_reporter/README.md
@@ -12,10 +12,10 @@ copy(`wget "${Array.from($$('a[href$=".xml"]')).filter(a => a.innerText === 'Dow
 
 This copies a script to download the reports, which you should execute in the `test/junit` directory.
 
-Next, run the CLI in `--dry-run` mode so that it doesn't actually communicate with Github.
+Next, run the CLI in `--no-github-update` mode so that it doesn't actually communicate with Github and `--no-report-update` to prevent the script from mutating the reports on disk and instead log the updated report.
 
 ```sh
-node scripts/report_failed_tests.js --verbose --dry-run --build-url foo
+node scripts/report_failed_tests.js --verbose --no-github-update --no-report-update
 ```
 
-If you specify the `GITHUB_TOKEN` environment variable then `--dry-run` will execute read operations but still won't execute write operations.
+Unless you specify the `GITHUB_TOKEN` environment variable requests to read existing issues will use anonymous access which is limited to 60 requests per hour.
diff --git a/packages/kbn-test/src/failed_tests_reporter/github_api.ts b/packages/kbn-test/src/failed_tests_reporter/github_api.ts
@@ -19,7 +19,7 @@
 
 import Url from 'url';
 
-import Axios, { AxiosRequestConfig } from 'axios';
+import Axios, { AxiosRequestConfig, AxiosInstance } from 'axios';
 import parseLinkHeader from 'parse-link-header';
 import { ToolingLog, isAxiosResponseError, isAxiosRequestError } from '@kbn/dev-utils';
 
@@ -40,25 +40,34 @@ type RequestOptions = AxiosRequestConfig & {
 };
 
 export class GithubApi {
- private readonly x = Axios.create({
- headers: {
- ...(this.token ? { Authorization: `token ${this.token}` } : {}),
- 'User-Agent': 'elastic/kibana#failed_test_reporter',
- },
- });
+ private readonly log: ToolingLog;
+ private readonly token: string | undefined;
+ private readonly dryRun: boolean;
+ private readonly x: AxiosInstance;
 
  /**
  * Create a GithubApi helper object, if token is undefined requests won't be
  * sent, but will instead be logged.
  */
- constructor(
- private readonly log: ToolingLog,
- private readonly token: string | undefined,
- private readonly dryRun: boolean
- ) {
- if (!token && !dryRun) {
+ constructor(options: {
+ log: GithubApi['log'];
+ token: GithubApi['token'];
+ dryRun: GithubApi['dryRun'];
+ }) {
+ this.log = options.log;
+ this.token = options.token;
+ this.dryRun = options.dryRun;
+
+ if (!this.token && !this.dryRun) {
  throw new TypeError('token parameter is required');
  }
+
+ this.x = Axios.create({
+ headers: {
+ ...(this.token ? { Authorization: `token ${this.token}` } : {}),
+ 'User-Agent': 'elastic/kibana#failed_test_reporter',
+ },
+ });
  }
 
  private failedTestIssuesPageCache: {

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
@@ -30,19 +30,14 @@ import { addMessagesToReport, Message } from './add_messages_to_report';
 export function runFailedTestsReporterCli() {
  run(
  async ({ log, flags }) => {
- const buildUrl = flags['build-url'];
- if (typeof buildUrl !== 'string' || !buildUrl) {
- throw createFlagError('Missing --build-url or process.env.BUILD_URL');
+ let updateGithub = flags['github-update'];
+ if (updateGithub && !process.env.GITHUB_TOKEN) {
+ throw createFailError(
+ 'GITHUB_TOKEN environment variable must be set, otherwise use --no-github-update flag'
+ );
  }
 
- let dryRun = !!flags['dry-run'];
- if (!dryRun) {
- if (!process.env.GITHUB_TOKEN) {
- throw createFailError(
- 'GITHUB_TOKEN environment variable must be set, otherwise use --dry-run flag'
- );
- }
-
+ if (updateGithub) {
  // JOB_NAME is formatted as `elastic+kibana+7.x` in some places and `elastic+kibana+7.x/JOB=kibana-intake,node=immutable` in others
  const jobNameSplit = (process.env.JOB_NAME || '').split(/\+|\//);
  const branch = jobNameSplit.length >= 3 ? jobNameSplit[2] : process.env.GIT_BRANCH;
@@ -56,14 +51,22 @@ export function runFailedTestsReporterCli() {
  const isMasterOrVersion =
  branch.match(/^(origin\/){0,1}master$/) || branch.match(/^(origin\/){0,1}\d+\.(x|\d+)$/);
  if (!isMasterOrVersion || isPr) {
- log.info(
- 'Failure issues only created on master/version branch jobs, switching to --dry-run mode'
- );
- dryRun = true;
+ log.info('Failure issues only created on master/version branch jobs');
+ updateGithub = false;
  }
  }
 
- const githubApi = new GithubApi(log, process.env.GITHUB_TOKEN, dryRun);
+ const githubApi = new GithubApi({
+ log,
+ token: process.env.GITHUB_TOKEN,
+ dryRun: !updateGithub,
+ });
+
+ const buildUrl = flags['build-url'] || (updateGithub ? '' : 'http://buildUrl');
+ if (typeof buildUrl !== 'string' || !buildUrl) {
+ throw createFlagError('Missing --build-url or process.env.BUILD_URL');
+ }
+
  const reportPaths = await globby(['target/junit/**/*.xml'], {
  cwd: REPO_ROOT,
  absolute: true,
@@ -80,7 +83,7 @@ export function runFailedTestsReporterCli() {
  name: failure.name,
  message:
  'Failure is likely irrelevant' +
- (dryRun ? '' : ', so an issue was not created or updated'),
+ (updateGithub ? ', so an issue was not created or updated' : ''),
  });
  continue;
  }
@@ -96,7 +99,9 @@ export function runFailedTestsReporterCli() {
  const url = existingIssue.html_url;
  const message =
  `Test has failed ${newFailureCount - 1} times on tracked branches: ${url}` +
- (dryRun ? '' : `. Updated existing issue: ${url} (fail count: ${newFailureCount})`);
+ (updateGithub
+ ? `. Updated existing issue: ${url} (fail count: ${newFailureCount})`
+ : '');
 
  messages.push({
  classname: failure.classname,
@@ -109,7 +114,7 @@ export function runFailedTestsReporterCli() {
  const newIssueUrl = await createFailureIssue(buildUrl, failure, githubApi);
  const message =
  `Test has not failed recently on tracked branches` +
- (dryRun ? '' : `Created new issue: ${newIssueUrl}`);
+ (updateGithub ? `Created new issue: ${newIssueUrl}` : '');
 
  messages.push({
  classname: failure.classname,
@@ -119,19 +124,28 @@ export function runFailedTestsReporterCli() {
  }
 
  // mutates report to include messages and writes updated report to disk
- await addMessagesToReport({ report, messages, log, reportPath, dryRun });
+ await addMessagesToReport({
+ report,
+ messages,
+ log,
+ reportPath,
+ dryRun: !flags['report-update'],
+ });
  }
  },
  {
  description: `a cli that opens issues or updates existing issues based on junit reports`,
  flags: {
- boolean: ['dry-run', 'skip-junit-update'],
+ boolean: ['github-update', 'report-update'],
  string: ['build-url'],
  default: {
+ 'github-update': true,
+ 'report-update': true,
  'build-url': process.env.BUILD_URL,
  },
  help: `
- --dry-run Execute the CLI without contacting Github
+ --no-github-update Execute the CLI without writing to Github
+ --no-report-update Execute the CLI without writing to the JUnit reports
  --build-url URL of the failed build, defaults to process.env.BUILD_URL
  `,
  },