-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tools: improve node:test
output when running in github actions
#49129
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,4 +94,4 @@ jobs: | |
- name: Test | ||
run: | | ||
cd $TAR_DIR | ||
make run-ci -j2 V=1 TEST_CI_ARGS="-p dots --measure-flakiness 9" | ||
make run-ci -j2 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=spec' --measure-flakiness 9" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the CI ran fine so I guess there is no issue, but are we sure this won't impact any of the tests that expect different reporters? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no issue. tests that look at reporters (e.g snapshot tests, exit code tests etc) always spawn a child and control the env vars and flags |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
#!/bin/sh | ||
|
||
# Shell script to update @reporters/github in the source tree to the latest release. | ||
|
||
# This script must be in the tools directory when it runs because it uses the | ||
# script source file path to determine directories to work in. | ||
|
||
set -ex | ||
|
||
ROOT=$(cd "$(dirname "$0")/../.." && pwd) | ||
[ -z "$NODE" ] && NODE="$ROOT/out/Release/node" | ||
[ -x "$NODE" ] || NODE=$(command -v node) | ||
NPM="$ROOT/deps/npm/bin/npm-cli.js" | ||
|
||
# shellcheck disable=SC1091 | ||
. "$ROOT/tools/dep_updaters/utils.sh" | ||
|
||
NEW_VERSION=$("$NODE" "$NPM" view @reporters/github dist-tags.latest) | ||
CURRENT_VERSION=$("$NODE" -p "require('./tools/github_reporter/package.json').version") | ||
|
||
# This function exit with 0 if new version and current version are the same | ||
compare_dependency_version "@reporters/github" "$NEW_VERSION" "$CURRENT_VERSION" | ||
|
||
cd "$( dirname "$0" )/../.." || exit | ||
rm -rf tools/github_reporter | ||
|
||
( | ||
rm -rf github-reporter-tmp | ||
mkdir github-reporter-tmp | ||
cd github-reporter-tmp || exit | ||
|
||
"$NODE" "$NPM" init --yes | ||
"$NODE" "$NPM" install --global-style --no-bin-links --ignore-scripts "@reporters/github@$NEW_VERSION" | ||
"$NODE" "$NPM" exec --package=esbuild@0.17.15 --yes -- esbuild ./node_modules/@reporters/github --bundle --platform=node --outfile=bundle.js | ||
) | ||
|
||
mkdir tools/github_reporter | ||
mv github-reporter-tmp/bundle.js tools/github_reporter/index.js | ||
mv github-reporter-tmp/node_modules/@reporters/github/package.json tools/github_reporter/package.json | ||
rm -rf github-reporter-tmp/ | ||
|
||
# The last line of the script should always print the new version, | ||
# as we need to add it to $GITHUB_ENV variable. | ||
echo "NEW_VERSION=$NEW_VERSION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be an improvement in terms of the amount of output we see from failed tests, which should help with debugging.
There is also the issue of the asan CI job truncating output. Since
node:test
is never used as the main test runner, a possible next step to address that issue could be to create a custom reporter intools/
that only outputs the test failures and diagnostic messages. It could basically be a slimmed down version of the spec reporter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can do that in a follow-up. perhaps only in the asan job