Enable C++ tests for the Emscripten build

[anutosh491]

Description

Enables C++ tests for the Emscripten build

Type of change

Please tick all options which are relevant.

• Bug fix
• New feature
• Added/removed dependencies
• Required documentation updates

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.56%. Comparing base (4ee2748) to head (d58786a).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #277   +/-   ##
=======================================
  Coverage   80.56%   80.56%           
=======================================
  Files          20       20           
  Lines         957      957           
  Branches       88       88           
=======================================
  Hits          771      771           
  Misses        186      186           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[anutosh491]

Currently we have around 45/47 test working for lite as compared to the native cases. According my surface level debug, these don't look like lite related errors, rather some internal functions going off I suppose :\ (So not something really major)

But yeah that being said this PR is just to get us started and I have marked the two tests as TODO and shall address them through subsequent prs.

I have tried to be as verbose as possible with the comments above and probably should make it easier to review.

cc @DerThorsten @JohanMabille could you possibly help me with the reviews here ?

Here are the logs https://github.com/compiler-research/xeus-cpp/actions/runs/13715851519/job/38360489317?pr=277#step:6:117

[vgvassilev]

We should merge this PR only after compiler-research/CppInterOp#483 is in.

[SylvainCorlay]

We should merge this PR only after compiler-research/CppInterOp#483 is in.

The two PRs seem fairly independent. We should move forward with this one IMO rather than creating artificial blockers.

[vgvassilev]

We should merge this PR only after compiler-research/CppInterOp#483 is in.

The two PRs seem fairly independent. We should move forward with this one IMO rather than creating artificial blockers.

Let’s discuss that on Monday.

[anutosh491]

@mcbarton I think you've had some doubts with flags (like -fexceptions, memory growth etc etc)

All are introduced to keep consistency with xcpp.wasm (which is the main module finally running and responsible for any output ... so obviously our test's main module should be following the same output)

Here you can find all the initital set of buildtime and link time flags that are responsible for building xcpp.wasm

https://github.com/jupyter-xeus/xeus/blob/main/cmake/WasmBuildOptions.cmake

[mcbarton]

@anutosh491 Don't we want the build and deploy workflow also to run these tests, so we can check the nightly build is working?

[mcbarton]

@mcbarton I think you've had some doubts with flags (like -fexceptions, memory growth etc etc)

All are introduced to keep consistency with xcpp.wasm (which is the main module finally running and responsible for any output ... so obviously our test's main module should be following the same output)

Here you can find all the initital set of buildtime and link time flags that are responsible for building xcpp.wasm

https://github.com/jupyter-xeus/xeus/blob/main/cmake/WasmBuildOptions.cmake

Unless specifically needed to enable and run the tests I don't think they should be part of this PR. I feel that if they are purely there for consistency purposes then they should be added in a separate PR. Are you able to run a commit which shows tests fail if they aren't included?

[anutosh491]

Unless specifically needed to enable and run the tests I don't think they should be part of this PR. I feel that if they are purely there for consistency purposes then they should be added in a separate PR. Are you able to run a commit which shows tests fail if they aren't included?

Why would that be ? We build xcpp.wasm with the same flags and we technically want to exactly replicate what we see with xeus-cpp-lite. Let's just keep them here. I don't see a point splitting them. I think they are essential.

[anutosh491]

Don't we want the build and deploy workflow also to run these tests, so we can check the nightly build is working?

Hmm, I don't know. This initial PR is to get us started. I think we're good with having tests for it in the CI right now. Let's extend it further if we think it might be really essential.

[anutosh491]

I don't see a point splitting them. I think they are essential.

okay I think when I say "essential", these are really essential. For example getting rid of the stack size/initial memory or changing them not to match xcpp.wasm gives me this

(xeus-lite-host) anutosh491@Anutoshs-MacBook-Air test % node test_xeus_cpp.js 
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
clang version 19.1.7 (https://github.com/emscripten-forge/recipes 9a1ec8d26a12571ea7d89a49434d24e2a2f00d72)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: 
Build config: +assertions
 "" -cc1 -triple wasm32-unknown-emscripten -emit-obj -disable-free -clear-ast-before-backend -main-file-name "<<< inputs >>>" -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/ -v -v -fcoverage-compilation-dir=/ -resource-dir /Users/anutosh491/micromamba/envs/xeus-cpp-wasm-host/lib/clang/19 -include new -internal-isystem /include/wasm32-emscripten/c++/v1 -internal-isystem /include/c++/v1 -internal-isystem /Users/anutosh491/micromamba/envs/xeus-cpp-wasm-host/lib/clang/19/include -internal-isystem /include/wasm32-emscripten -internal-isystem /include -std=c++20 -fdeprecated-macro -ferror-limit 19 -fvisibility=default -fgnuc-version=4.2.1 -fno-implicit-modules -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -fincremental-extensions -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>"
clang -cc1 version 19.1.7 based upon LLVM 19.1.7 default target wasm32-unknown-emscripten
ignoring nonexistent directory "/include/wasm32-emscripten/c++/v1"
ignoring nonexistent directory "/Users/anutosh491/micromamba/envs/xeus-cpp-wasm-host/lib/clang/19/include"
ignoring nonexistent directory "/include/wasm32-emscripten"
#include "..." search starts here:
#include <...> search starts here:
 /include/c++/v1
 /include
End of search list.
wasm://wasm/00a92bfe:1


RuntimeError: memory access out of bounds
    at wasm://wasm/00a92bfe:wasm-function[3086]:0x1bab7a
    at wasm://wasm/00a92bfe:wasm-function[3913]:0x1d9bce
    at libclangCppInterOp.so.std::__2::__tree<std::__2::__value_type<clang::QualType, clang::CXXBaseSpecifier*>, std::__2::__map_value_compare<clang::QualType, std::__2::__value_type<clang::QualType, clang::CXXBaseSpecifier*>, clang::QualTypeOrdering, true>, std::__2::allocator<std::__2::__value_type<clang::QualType, clang::CXXBaseSpecifier*>>>::destroy(std::__2::__tree_node<std::__2::__value_type<clang::QualType, clang::CXXBaseSpecifier*>, void*>*) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[15584]:0x115f733)
    at libclangCppInterOp.so.clang::Sema::AttachBaseSpecifiers(clang::CXXRecordDecl*, llvm::MutableArrayRef<clang::CXXBaseSpecifier*>) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[15582]:0x115f59c)
    at libclangCppInterOp.so.clang::Sema::InstantiateClass(clang::SourceLocation, clang::CXXRecordDecl*, clang::CXXRecordDecl*, clang::MultiLevelTemplateArgumentList const&, clang::TemplateSpecializationKind, bool) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[19427]:0x15d7cd7)
    at libclangCppInterOp.so.clang::Sema::InstantiateClassTemplateSpecialization(clang::SourceLocation, clang::ClassTemplateSpecializationDecl*, clang::TemplateSpecializationKind, bool) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[19433]:0x15d9707)
    at libclangCppInterOp.so.void llvm::function_ref<void ()>::callback_fn<clang::Sema::RequireCompleteTypeImpl(clang::SourceLocation, clang::QualType, clang::Sema::CompleteTypeKind, clang::Sema::TypeDiagnoser*)::$_0>(long) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[19850]:0x166071e)
    at libclangCppInterOp.so.clang::Sema::runWithSufficientStackSpace(clang::SourceLocation, llvm::function_ref<void ()>) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[13559]:0xf9512f)
    at libclangCppInterOp.so.clang::Sema::RequireCompleteTypeImpl(clang::SourceLocation, clang::QualType, clang::Sema::CompleteTypeKind, clang::Sema::TypeDiagnoser*) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[19781]:0x16494d4)
    at libclangCppInterOp.so.clang::Sema::RequireCompleteType(clang::SourceLocation, clang::QualType, clang::Sema::CompleteTypeKind, clang::Sema::TypeDiagnoser&) (wasm://wasm/libclangCppInterOp.so-10edf8ce:wasm-function[19765]:0x16465d6)

Node.js v23.9.0

[mcbarton]

@anutosh491 Please update the documentation as part of this PR so that developers know that xeus-cpp has Emscripten tests, and how to run them.

[anutosh491]

This should be skipped rather than ignored completely I believe. This one issue for curl was labelled as won't fix, but by bot emscripten-core/emscripten#4906 . This issue emscripten-core/emscripten#13025 that got a reply didn't get a response that curl doesn't work, just that sockets is very limited. Even going away from curl xeus-cpp could make use of Emscriptens fetch API https://emscripten.org/docs/api_reference/fetch.html .

Hi,

Let's not stress too much on this. Not sure what you mean by completely ignore because I've just disabled it for the emscripten build cause currently we don't have a way to get this work.

Anyways to get any AI support in the browser, I think we would just go the JupyterliteAI route when that's ready.

[anutosh491]

Hey @vgvassilev @JohanMabille

I think this should be ready now. Apart from the 2 tests that have been added as TODOs rest all work (45/47) so really happy with the results. This should be good to get us started I think.

[mcbarton]

Hey @vgvassilev @JohanMabille

I think this should be ready now. Apart from the 2 tests that have been added as TODOs rest all work (45/47) so really happy with the results. This should be good to get us started I think.

@vgvassilev @JohanMabille This comment should be addressed before any approval is given #277 (comment) . I don't believe xeus-cpp should rely on Emscripten forge setting LD_FLAGS. Where possible in the build process we should maintain compatibility with the upstream emsdk.

[anutosh491]

I don't believe xeus-cpp should rely on Emscripten forge setting LD_FLAGS

Hi
Once the move to emscripten 3.1.73 all xeus lite kernels currently stick to use emsdk from emscripten-forge where WASM_BIGINT is a default link flag along with some others.

Where possible in the build process we should maintain compatibility with the upstream emsdk.

I don't think xeus-cpp or any other xeus-lite kernel (or for that matter any Jupyterlite kernel) has ever claimed this. I don't think this is a hard necessity.

[anutosh491]

I've made the change to keep everyone happy !

Should be good to go now.

[mcbarton]

LGTM

[anutosh491]

Hey @vgvassilev this should be ready now. Could you please have a look and merge if you're happy ?