-
Notifications
You must be signed in to change notification settings - Fork 74
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
Add zstd
for wasm build
#641
Conversation
@martinRenou @JohanMabille do you know if the failed tests are related to my changes? |
It seems the error comes out of this test xeus-python/test/test_debugger.cpp Line 1246 in 8725149
If you need this to go in, maybe you could comment this and add a TODO ? |
It seems some specific operations might be going wrong (first @trungleduc could you check and add todo's for the failing ones. I shall look into them once that is done ! |
I guess we can do what you did but what I meant was to only comment out the tests that fail (I guess a couple would fail) and add todos for only what fails Otherwise a better way to do this might be to comment this out and add a todo xeus-python/.github/workflows/main.yml Lines 71 to 74 in 8725149
And these out xeus-python/test/CMakeLists.txt Lines 53 to 55 in 8725149
So that we don't end up with this and we know the tests need to be fixed.
|
ff8fca0
to
8b59978
Compare
@anutosh491 is it ok now? |
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.
The failures of the debugger tests are not related to your changes. I think we can merge this one and investigate the failing tests in a dedicated PR.
Needed for emscripten-forge/pyjs#76