Repository navigation
Add CPython to CI - #28113
Add CPython to CI#28113hoodmane wants to merge 5 commits into
Conversation
Resolves issue emscripten-core#28097.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new CircleCI job and a supporting Python script (test/cpython_test.py) to build and run the CPython test suite against the local Emscripten checkout. The review feedback focuses on improving the robustness of the script: adding safety checks for config.NODE_JS in node_path(), preventing a trailing path separator when modifying PATH in build_env(), simplifying the test() function by removing redundant exception handling, and catching subprocess.CalledProcessError at the top level in main() to exit cleanly without tracebacks.
6cc8b2f to
395403d
Compare
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
Nice!
Sorry about new noise from gemini code review... sometimes is get be useful, but maybe not here.
| sys.path.insert(0, __rootpath__) | ||
|
|
||
| from tools import config | ||
| from tools.utils import path_from_root |
There was a problem hiding this comment.
Just FYI, I'm trying to move away from using parts of the compiler itself in test code like this.
This looks pretty minimal but the more you can avoid using these the better (since in the long run I'd like to remove this code sharing).
| # we always want to use the local emscripten. | ||
| env.pop('EMSDK_CACHE', None) | ||
| if config.EM_CONFIG: | ||
| env['EM_CONFIG'] = config.EM_CONFIG |
There was a problem hiding this comment.
I believe this is already done in config.py:init (on line 274)
|
Thanks @sbc100 for the quick review! |
Resolves issue #28097.