mirror of
https://github.com/elastic/kibana.git
synced 2025-06-28 03:01:21 -04:00
Fix typecheck foundations (#167060)
## Summary This PR is the core part of #166813. The original work seems to grow large, and we'd like to enable a preventive check beforehand to prevent more errors from entering the codebase. The idea is to have a selective type check that would only check changed files' projects. - [x] when there's no extra label, run the selective type check only on the diffing files' projects (success: https://buildkite.com/elastic/kibana-pull-request/builds/161837) - [x] when the label `ci:hard-typecheck` is present, run the regular (but now, working) full typecheck (expected to fail: ) cc: @watson --------- Co-authored-by: Brad White <brad.white@elastic.co> Co-authored-by: Thomas Watson <w@tson.dk> Co-authored-by: Thomas Watson <watson@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
parent
4662960980
commit
e81728ee96
10 changed files with 181 additions and 16 deletions
|
@ -136,12 +136,13 @@ steps:
|
||||||
- exit_status: '-1'
|
- exit_status: '-1'
|
||||||
limit: 3
|
limit: 3
|
||||||
|
|
||||||
- command: .buildkite/scripts/steps/check_types.sh
|
# TODO: Enable in #166813 after fixing types
|
||||||
label: 'Check Types'
|
# - command: .buildkite/scripts/steps/check_types.sh
|
||||||
agents:
|
# label: 'Check Types'
|
||||||
queue: n2-16-spot
|
# agents:
|
||||||
timeout_in_minutes: 60
|
# queue: n2-16-spot
|
||||||
retry:
|
# timeout_in_minutes: 60
|
||||||
automatic:
|
# retry:
|
||||||
- exit_status: '-1'
|
# automatic:
|
||||||
limit: 3
|
# - exit_status: '-1'
|
||||||
|
# limit: 3
|
||||||
|
|
10
.buildkite/pipelines/pull_request/type_check.yml
Normal file
10
.buildkite/pipelines/pull_request/type_check.yml
Normal file
|
@ -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
.buildkite/pipelines/pull_request/type_check_selective.yml
Normal file
10
.buildkite/pipelines/pull_request/type_check_selective.yml
Normal file
|
@ -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
|
|
@ -59,6 +59,12 @@ const uploadPipeline = (pipelineContent: string | object) => {
|
||||||
pipeline.push(getPipeline('.buildkite/pipelines/pull_request/kbn_handlebars.yml'));
|
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 (
|
if (
|
||||||
(await doAnyChangesMatch([
|
(await doAnyChangesMatch([
|
||||||
/^src\/plugins\/controls/,
|
/^src\/plugins\/controls/,
|
||||||
|
|
|
@ -4,8 +4,9 @@ set -euo pipefail
|
||||||
|
|
||||||
.buildkite/scripts/bootstrap.sh
|
.buildkite/scripts/bootstrap.sh
|
||||||
|
|
||||||
echo "--- Run scripts/type_check to ensure that all build available"
|
# TODO: Enable in #166813 after fixing types
|
||||||
node scripts/type_check
|
# echo "--- Run scripts/type_check to ensure that all build available"
|
||||||
|
# node scripts/type_check
|
||||||
|
|
||||||
echo "--- Build API Docs"
|
echo "--- Build API Docs"
|
||||||
node --max-old-space-size=12000 scripts/build_api_docs
|
node --max-old-space-size=12000 scripts/build_api_docs
|
||||||
|
|
114
.buildkite/scripts/steps/check_types_commits.sh
Normal file
114
.buildkite/scripts/steps/check_types_commits.sh
Normal file
|
@ -0,0 +1,114 @@
|
||||||
|
#!/usr/bin/env bash
|
||||||
|
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
|
||||||
|
if [[ "${CI-}" == "true" ]]; then
|
||||||
|
.buildkite/scripts/bootstrap.sh
|
||||||
|
|
||||||
|
sha1="${GITHUB_PR_TARGET_BRANCH-}"
|
||||||
|
sha2="${GITHUB_PR_TRIGGERED_SHA-}"
|
||||||
|
else
|
||||||
|
# Script take between 0 and 2 arguments representing two commit SHA's:
|
||||||
|
# If 0, it will diff HEAD and HEAD^
|
||||||
|
# If 1 (SHA1), it will diff SHA1 and SHA1^
|
||||||
|
# If 2 (SHA1, SHA2), it will diff SHA1 and SHA2
|
||||||
|
sha1="${1-HEAD}"
|
||||||
|
sha2="${2-$sha1^}"
|
||||||
|
fi
|
||||||
|
|
||||||
|
uniq_dirs=()
|
||||||
|
uniq_tsconfigs=()
|
||||||
|
|
||||||
|
echo "Detecting files changed between $sha1 and $sha2..."
|
||||||
|
|
||||||
|
files=($(git diff --name-only $sha1 $sha2))
|
||||||
|
|
||||||
|
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..."
|
||||||
|
|
||||||
|
for dir in "${uniq_dirs[@]}"
|
||||||
|
do
|
||||||
|
find_tsconfig $dir
|
||||||
|
done
|
||||||
|
|
||||||
|
if [ ${#uniq_tsconfigs[@]} -eq 0 ]; then
|
||||||
|
echo "No tsconfig.json files found for changes in $sha1 $sha2"
|
||||||
|
exit
|
||||||
|
fi
|
||||||
|
|
||||||
|
echo "Running scripts/type_check for each found tsconfig.json file..."
|
||||||
|
|
||||||
|
for tsconfig in "${uniq_tsconfigs[@]}"
|
||||||
|
do
|
||||||
|
node scripts/type_check --project $tsconfig
|
||||||
|
done
|
|
@ -66,7 +66,8 @@
|
||||||
"test:ftr:runner": "node scripts/functional_test_runner",
|
"test:ftr:runner": "node scripts/functional_test_runner",
|
||||||
"test:ftr:server": "node scripts/functional_tests_server",
|
"test:ftr:server": "node scripts/functional_tests_server",
|
||||||
"test:jest": "node scripts/jest",
|
"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": {
|
"repository": {
|
||||||
"type": "git",
|
"type": "git",
|
||||||
|
|
|
@ -13,7 +13,7 @@ import stripAnsi from 'strip-ansi';
|
||||||
|
|
||||||
import execa from 'execa';
|
import execa from 'execa';
|
||||||
import * as Rx from 'rxjs';
|
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 chalk from 'chalk';
|
||||||
import treeKill from 'tree-kill';
|
import treeKill from 'tree-kill';
|
||||||
import { ToolingLog } from '@kbn/tooling-log';
|
import { ToolingLog } from '@kbn/tooling-log';
|
||||||
|
@ -87,13 +87,23 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) {
|
||||||
|
|
||||||
const outcome$: Rx.Observable<number | null> = Rx.race(
|
const outcome$: Rx.Observable<number | null> = Rx.race(
|
||||||
// observe first exit event
|
// 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),
|
take(1),
|
||||||
map(([code]) => {
|
map(([code]) => {
|
||||||
if (stopCalled) {
|
if (stopCalled) {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
// JVM exits with 143 on SIGTERM and 130 on SIGINT, dont' treat then as errors
|
|
||||||
|
// JVM exits with 143 on SIGTERM and 130 on SIGINT, don't treat them as errors
|
||||||
if (code > 0 && !(code === 143 || code === 130)) {
|
if (code > 0 && !(code === 143 || code === 130)) {
|
||||||
throw createFailError(`[${name}] exited with code ${code}`, {
|
throw createFailError(`[${name}] exited with code ${code}`, {
|
||||||
exitCode: code,
|
exitCode: code,
|
||||||
|
@ -108,6 +118,12 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) {
|
||||||
Rx.fromEvent(childProcess, 'error').pipe(
|
Rx.fromEvent(childProcess, 'error').pipe(
|
||||||
take(1),
|
take(1),
|
||||||
mergeMap((err) => Rx.throwError(err))
|
mergeMap((err) => Rx.throwError(err))
|
||||||
|
),
|
||||||
|
|
||||||
|
// observe a promise rejection
|
||||||
|
Rx.from(childProcess).pipe(
|
||||||
|
take(1),
|
||||||
|
mergeMap((err) => Rx.throwError(err))
|
||||||
)
|
)
|
||||||
).pipe(share());
|
).pipe(share());
|
||||||
|
|
||||||
|
@ -132,7 +148,7 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) {
|
||||||
share()
|
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) {
|
async function stop(signal: NodeJS.Signals) {
|
||||||
if (stopCalled) {
|
if (stopCalled) {
|
||||||
|
|
|
@ -135,6 +135,9 @@ export class ProcRunner {
|
||||||
// wait for process to complete
|
// wait for process to complete
|
||||||
await proc.outcomePromise;
|
await proc.outcomePromise;
|
||||||
}
|
}
|
||||||
|
} catch (e) {
|
||||||
|
this.log.error(e);
|
||||||
|
throw e;
|
||||||
} finally {
|
} finally {
|
||||||
// while the procRunner closes promises will resolve/reject because
|
// while the procRunner closes promises will resolve/reject because
|
||||||
// processes and stopping, but consumers of run() shouldn't have to
|
// processes and stopping, but consumers of run() shouldn't have to
|
||||||
|
|
|
@ -124,6 +124,9 @@ run(
|
||||||
'--pretty',
|
'--pretty',
|
||||||
...(flagsReader.boolean('verbose') ? ['--verbose'] : []),
|
...(flagsReader.boolean('verbose') ? ['--verbose'] : []),
|
||||||
],
|
],
|
||||||
|
env: {
|
||||||
|
NODE_OPTIONS: '--max-old-space-size=8192',
|
||||||
|
},
|
||||||
cwd: REPO_ROOT,
|
cwd: REPO_ROOT,
|
||||||
wait: true,
|
wait: true,
|
||||||
});
|
});
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue