Skip to content

Commit

Permalink
[8.10] Fix typecheck foundations (#167060) (#167394)
Browse files Browse the repository at this point in the history
Co-authored-by: Brad White <brad.white@elastic.co>
  • Loading branch information
Thomas Watson and Ikuni17 authored Sep 28, 2023
1 parent 3f9d09d commit ae8abdd
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 15 deletions.
19 changes: 10 additions & 9 deletions .buildkite/pipelines/pull_request/base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 10 additions & 0 deletions .buildkite/pipelines/pull_request/type_check.yml
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions .buildkite/pipelines/pull_request/type_check_selective.yml
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions .buildkite/scripts/pipelines/pull_request/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/,
Expand Down
5 changes: 3 additions & 2 deletions .buildkite/scripts/steps/build_api_docs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
165 changes: 165 additions & 0 deletions .buildkite/scripts/steps/check_types_commits.sh
Original file line number Diff line number Diff line change
@@ -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 [<ref1> [<ref2>]]"
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
22 changes: 19 additions & 3 deletions packages/kbn-dev-proc-runner/src/proc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -87,12 +87,22 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) {

const outcome$: Rx.Observable<number | null> = 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}`, {
Expand All @@ -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());

Expand All @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-dev-proc-runner/src/proc_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-ts-type-check-cli/run_type_check_cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ run(
'--pretty',
...(flagsReader.boolean('verbose') ? ['--verbose'] : []),
],
env: {
NODE_OPTIONS: '--max-old-space-size=8192',
},
cwd: REPO_ROOT,
wait: true,
});
Expand Down

0 comments on commit ae8abdd

Please sign in to comment.