-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add test runner option --log-test-environment #25843
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
Conversation
…and print detailed information about Emscripten tool setup: Emcc, Clang, Binaryen, Node, Python and LLVM. This helps CI maintainers emit info to test run logs of the active setup for reference.
sbc100
left a comment
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.
Seems like useful thing!
| print(f'\n{utils.read_file(config.EM_CONFIG).strip()}\n') | ||
|
|
||
| node_js_version = subprocess.run(config.NODE_JS + ['--version'], stdout=subprocess.PIPE, text=True).stdout.strip() | ||
| print(f'NODE_JS: {config.NODE_JS}. Version: {node_js_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.
We already have this available via utils.get_node_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.
There's shared.get_node_version(), but that parses the output. I think it's better in this case to output the raw version string as emitted by NODE_JS --version.
| print_repository_info(llvm_git_root, 'LLVM') | ||
|
|
||
| clang_version = subprocess.run([shared.CLANG_CC, '--version'], stdout=subprocess.PIPE, text=True).stdout.strip() | ||
| print(f'Clang: "{shared.CLANG_CC}"\n{clang_version}\n') |
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.
We have shared.get_clang_version() for this.
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.
That too parses the output - it would be preferable to print the raw output here.
test/runner.py
Outdated
| import subprocess | ||
| current_commit = subprocess.run(['git', 'log', '-n1'], cwd=directory, stdout=subprocess.PIPE, text=True).stdout.strip() | ||
| print(f'\n{repository_name} {current_commit}\n') | ||
| local_changes = subprocess.run(['git', 'diff'], cwd=directory, stdout=subprocess.PIPE, text=True).stdout.strip() |
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.
Maybe just remove the stdout=subprocess.PIPE so that the output goes directly to stdout?
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 doesn't work for Windows at least, but enters an interactive git diff prompt.
test/runner.py
Outdated
|
|
||
| browser_common.init(options.force_browser_process_termination) | ||
|
|
||
| if options.log_test_environment: |
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 should probably print all this into in circle CI too. The best way to do that is probably to add or os.getenv('CI').
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.
Sounds good. Added os.getenv('CI').
|
Do we really need show the whole commit messsage from at HEAD for each repo? Isn't the revision enough? The commit message seems to add a log noise to me. |
|
I thought it would be a good way to read out what the current tested commit was. When I see a list of test runs on my CI (runs in newest to oldest order), I could look at the first red run, and then read the commit message there - that would give a direct PR #number to find what regressed. If I only printed a commit hash, I would then need to go and |
I think the commit has is normally enough for me, but maybe can do |
|
Ok, that works. |
Add test runner option --log-test-environment, which will introspect and print detailed information about Emscripten tool setup: Emcc, Clang, Binaryen, Node, Python and LLVM. This helps CI maintainers emit info to test run logs of the active setup for reference.
test/runner --log-test-environment <tests_to_run>will then print a following log info dump before starting the tests: