From ae8abdd0a23a385646b358db74e78d2abfbfefb9 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Thu, 28 Sep 2023 11:41:02 +0200 Subject: [PATCH] [8.10] Fix typecheck foundations (#167060) (#167394) Co-authored-by: Brad White --- .buildkite/pipelines/pull_request/base.yml | 19 +- .../pipelines/pull_request/type_check.yml | 10 ++ .../pull_request/type_check_selective.yml | 10 ++ .../pipelines/pull_request/pipeline.ts | 6 + .buildkite/scripts/steps/build_api_docs.sh | 5 +- .../scripts/steps/check_types_commits.sh | 165 ++++++++++++++++++ package.json | 3 +- packages/kbn-dev-proc-runner/src/proc.ts | 22 ++- .../kbn-dev-proc-runner/src/proc_runner.ts | 3 + .../run_type_check_cli.ts | 3 + 10 files changed, 231 insertions(+), 15 deletions(-) create mode 100644 .buildkite/pipelines/pull_request/type_check.yml create mode 100644 .buildkite/pipelines/pull_request/type_check_selective.yml create mode 100755 .buildkite/scripts/steps/check_types_commits.sh diff --git a/.buildkite/pipelines/pull_request/base.yml b/.buildkite/pipelines/pull_request/base.yml index 86c94dad7a667..91b508d7f87d7 100644 --- a/.buildkite/pipelines/pull_request/base.yml +++ b/.buildkite/pipelines/pull_request/base.yml @@ -88,12 +88,13 @@ steps: - exit_status: '-1' limit: 3 - - command: .buildkite/scripts/steps/check_types.sh - label: 'Check Types' - agents: - queue: n2-16-spot - timeout_in_minutes: 60 - retry: - automatic: - - exit_status: '-1' - limit: 3 + # TODO: Enable in #166813 after fixing types + # - command: .buildkite/scripts/steps/check_types.sh + # label: 'Check Types' + # agents: + # queue: n2-16-spot + # timeout_in_minutes: 60 + # retry: + # automatic: + # - exit_status: '-1' + # limit: 3 diff --git a/.buildkite/pipelines/pull_request/type_check.yml b/.buildkite/pipelines/pull_request/type_check.yml new file mode 100644 index 0000000000000..dc6f8471ff1b2 --- /dev/null +++ b/.buildkite/pipelines/pull_request/type_check.yml @@ -0,0 +1,10 @@ +steps: + - command: .buildkite/scripts/steps/check_types.sh + label: 'Check Types' + agents: + queue: n2-16-spot + timeout_in_minutes: 60 + retry: + automatic: + - exit_status: '-1' + limit: 3 diff --git a/.buildkite/pipelines/pull_request/type_check_selective.yml b/.buildkite/pipelines/pull_request/type_check_selective.yml new file mode 100644 index 0000000000000..7d01f128aac3c --- /dev/null +++ b/.buildkite/pipelines/pull_request/type_check_selective.yml @@ -0,0 +1,10 @@ +steps: + - command: .buildkite/scripts/steps/check_types_commits.sh + label: 'Check Types Commit Diff' + agents: + queue: n2-16-spot + timeout_in_minutes: 60 + retry: + automatic: + - exit_status: '-1' + limit: 3 diff --git a/.buildkite/scripts/pipelines/pull_request/pipeline.ts b/.buildkite/scripts/pipelines/pull_request/pipeline.ts index 72bdc3358e58f..c78a13b7a03b7 100644 --- a/.buildkite/scripts/pipelines/pull_request/pipeline.ts +++ b/.buildkite/scripts/pipelines/pull_request/pipeline.ts @@ -59,6 +59,12 @@ const uploadPipeline = (pipelineContent: string | object) => { pipeline.push(getPipeline('.buildkite/pipelines/pull_request/kbn_handlebars.yml')); } + if (GITHUB_PR_LABELS.includes('ci:hard-typecheck')) { + pipeline.push(getPipeline('.buildkite/pipelines/pull_request/type_check.yml')); + } else { + pipeline.push(getPipeline('.buildkite/pipelines/pull_request/type_check_selective.yml')); + } + if ( (await doAnyChangesMatch([ /^src\/plugins\/controls/, diff --git a/.buildkite/scripts/steps/build_api_docs.sh b/.buildkite/scripts/steps/build_api_docs.sh index f86032c902d1a..ba1c2ec7ec94d 100755 --- a/.buildkite/scripts/steps/build_api_docs.sh +++ b/.buildkite/scripts/steps/build_api_docs.sh @@ -4,8 +4,9 @@ set -euo pipefail .buildkite/scripts/bootstrap.sh -echo "--- Run scripts/type_check to ensure that all build available" -node scripts/type_check +# TODO: Enable in #166813 after fixing types +# echo "--- Run scripts/type_check to ensure that all build available" +# node scripts/type_check echo "--- Build API Docs" node --max-old-space-size=12000 scripts/build_api_docs diff --git a/.buildkite/scripts/steps/check_types_commits.sh b/.buildkite/scripts/steps/check_types_commits.sh new file mode 100755 index 0000000000000..2d53e9e245929 --- /dev/null +++ b/.buildkite/scripts/steps/check_types_commits.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash + +set -euo pipefail + +argv=( "$@" ) +diffArgs=("--name-only") +uniq_dirs=() +uniq_tsconfigs=() + +is_flag_set () { + flag=$1 + if [ ${#argv[@]} -gt 0 ] && [[ ${argv[@]} =~ $flag ]]; then + true + else + false + fi +} + +if is_flag_set "--help" || is_flag_set "-h"; then + echo "Detects the files changed in a given set of commits, finds the related" + echo "tsconfig.json files, and scope the TypeScript type check to those." + echo + echo "Usage:" + echo " $0 [options]" + echo " $0 [ref1 [ref2]]" + echo + echo "Options:" + echo " --help, -h Show this help" + echo " --cached Check staged changes" + echo " --merge-base [ []]" + echo " Check changes between nearest common ansestor (merge-base) of" + echo " ref1 and ref2. Defaults: 'main' and 'HEAD'" + echo + echo "If no options are provided, the script takes between 0 and 2 arguments" + echo "representing two git refs:" + echo " If 0, it will diff HEAD and HEAD^" + echo " If 1 (REF1), it will diff REF1 and REF1^" + echo " If 2 (REF1, REF2), it will diff REF1 and REF2" + exit +fi + +if [[ "${CI-}" == "true" ]]; then + # Buildkite only + .buildkite/scripts/bootstrap.sh + + targetBranch="${GITHUB_PR_TARGET_BRANCH-}" + git fetch origin $targetBranch + sha=$(git merge-base "origin/$targetBranch" "${GITHUB_PR_TRIGGERED_SHA-}") + diffArgs+=("$sha" "${GITHUB_PR_TRIGGERED_SHA-}") +elif is_flag_set "--merge-base"; then + # Similar to when CI=true, but locally + diffArgs+=("--merge-base" "${2-main}" "${3-HEAD}") +elif is_flag_set "--cached"; then + # Only check staged files + diffArgs+=("--cached") +else + # Full manual mode! + ref1="${1-HEAD}" + diffArgs+=("$ref1" "${2-$ref1^}") +fi + +echo "Detecting files changed..." +echo "DEBUG: git diff args: ${diffArgs[@]}" +files=($(git diff "${diffArgs[@]}")) + +add_dir () { + new_dir=$1 + + if [ ${#uniq_dirs[@]} -gt 0 ]; then + for dir in "${uniq_dirs[@]}" + do + if [[ "$new_dir" == "$dir" ]]; then + return + fi + done + fi + + uniq_dirs+=($new_dir) +} + +add_tsconfig () { + new_tsconfig=$1 + + if [ ${#uniq_tsconfigs[@]} -gt 0 ]; then + for tsconfig in "${uniq_tsconfigs[@]}" + do + if [[ "$new_tsconfig" == "$tsconfig" ]]; then + return + fi + done + fi + + echo " $new_tsconfig" + uniq_tsconfigs+=($new_tsconfig) +} + +contains_tsconfig () { + dir=$1 + tsconfig="$dir/tsconfig.json" + if [ -f "$tsconfig" ]; then + true + else + false + fi +} + +find_tsconfig () { + dir=$1 + + if [[ "$dir" == "." ]]; then + return + fi + + if contains_tsconfig $dir; then + add_tsconfig "$dir/tsconfig.json" + else + find_tsconfig $(dirname -- "$dir") + fi +} + +if [ ${#files[@]} -eq 0 ]; then + echo "No files found!" + exit +fi + +for file in "${files[@]}" +do + dir=$(dirname -- "$file") + + # Ignore buildkite dir because it traverses many kbn packages and emits incorrect results + if [[ "$dir" != .buildkite* ]]; then + add_dir $dir + fi +done + +echo "Looking for related tsconfig.json files..." + +if [ ${#uniq_dirs[@]} -gt 0 ]; then + for dir in "${uniq_dirs[@]}" + do + find_tsconfig $dir + done +fi + +if [ ${#uniq_tsconfigs[@]} -eq 0 ]; then + echo "No tsconfig.json files found" + exit +fi + +echo "Running scripts/type_check for each found tsconfig.json file..." + +finalExitCode=0 + +for tsconfig in "${uniq_tsconfigs[@]}" +do + set +e + node scripts/type_check --project $tsconfig + exitCode=$? + set -e + if [ "$exitCode" -gt "$finalExitCode" ]; then + finalExitCode=$exitCode + fi +done + +exit $finalExitCode diff --git a/package.json b/package.json index 7f35e37f44361..d70dd5aac1ba2 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,8 @@ "test:ftr:runner": "node scripts/functional_test_runner", "test:ftr:server": "node scripts/functional_tests_server", "test:jest": "node scripts/jest", - "test:jest_integration": "node scripts/jest_integration" + "test:jest_integration": "node scripts/jest_integration", + "test:type_check": "node scripts/type_check" }, "repository": { "type": "git", diff --git a/packages/kbn-dev-proc-runner/src/proc.ts b/packages/kbn-dev-proc-runner/src/proc.ts index d30a893ae4c75..5b2b8ccfd7431 100644 --- a/packages/kbn-dev-proc-runner/src/proc.ts +++ b/packages/kbn-dev-proc-runner/src/proc.ts @@ -13,7 +13,7 @@ import stripAnsi from 'strip-ansi'; import execa from 'execa'; import * as Rx from 'rxjs'; -import { tap, share, take, mergeMap, map, ignoreElements } from 'rxjs/operators'; +import { tap, share, take, mergeMap, map, ignoreElements, filter } from 'rxjs/operators'; import chalk from 'chalk'; import treeKill from 'tree-kill'; import { ToolingLog } from '@kbn/tooling-log'; @@ -87,12 +87,22 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) { const outcome$: Rx.Observable = Rx.race( // observe first exit event - Rx.fromEvent<[number]>(childProcess, 'exit').pipe( + Rx.fromEvent<[number, string]>(childProcess, 'exit').pipe( + filter(([code]) => { + if (stopCalled) { + // when stop was already called, that's a graceful exit, let those events pass. + return true; + } else { + // filtering out further interruption events to prevent `take()` from closing the stream. + return code !== null; + } + }), take(1), map(([code]) => { if (stopCalled) { return null; } + // JVM exits with 143 on SIGTERM and 130 on SIGINT, dont' treat then as errors if (code > 0 && !(code === 143 || code === 130)) { throw createFailError(`[${name}] exited with code ${code}`, { @@ -108,6 +118,12 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) { Rx.fromEvent(childProcess, 'error').pipe( take(1), mergeMap((err) => Rx.throwError(err)) + ), + + // observe a promise rejection + Rx.from(childProcess).pipe( + take(1), + mergeMap((err) => Rx.throwError(err)) ) ).pipe(share()); @@ -132,7 +148,7 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) { share() ); - const outcomePromise = Rx.merge(lines$.pipe(ignoreElements()), outcome$).toPromise(); + const outcomePromise = Rx.firstValueFrom(Rx.merge(lines$.pipe(ignoreElements()), outcome$)); async function stop(signal: NodeJS.Signals) { if (stopCalled) { diff --git a/packages/kbn-dev-proc-runner/src/proc_runner.ts b/packages/kbn-dev-proc-runner/src/proc_runner.ts index 1226cbeb3eef1..510a96dc043be 100644 --- a/packages/kbn-dev-proc-runner/src/proc_runner.ts +++ b/packages/kbn-dev-proc-runner/src/proc_runner.ts @@ -135,6 +135,9 @@ export class ProcRunner { // wait for process to complete await proc.outcomePromise; } + } catch (e) { + this.log.error(e); + throw e; } finally { // while the procRunner closes promises will resolve/reject because // processes and stopping, but consumers of run() shouldn't have to diff --git a/packages/kbn-ts-type-check-cli/run_type_check_cli.ts b/packages/kbn-ts-type-check-cli/run_type_check_cli.ts index 0295525578952..3f4385118c9c6 100644 --- a/packages/kbn-ts-type-check-cli/run_type_check_cli.ts +++ b/packages/kbn-ts-type-check-cli/run_type_check_cli.ts @@ -124,6 +124,9 @@ run( '--pretty', ...(flagsReader.boolean('verbose') ? ['--verbose'] : []), ], + env: { + NODE_OPTIONS: '--max-old-space-size=8192', + }, cwd: REPO_ROOT, wait: true, });