-
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
base: main
Are you sure you want to change the base?
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.
| print_repository_info(llvm_git_root, 'LLVM') | ||
|
|
||
| clang_version = subprocess.run([shared.CLANG_CC, '--version'], stdout=subprocess.PIPE, text=True).stdout.strip() | ||
| clang_version = utils.run_process([shared.CLANG_CC, '--version'], 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.
You don't need text=True here, or above
|
|
||
| if os.path.isdir(os.path.join(config.EMSCRIPTEN_ROOT, '.git')): | ||
| print(f'\nEmscripten repository: "{config.EMSCRIPTEN_ROOT}"') | ||
| if os.path.isdir(os.path.join(__rootpath__, '.git')): |
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.
utils.path_from_root('.git')
|
|
||
| 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').
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: