snitch-org / snitch

Lightweight C++20 testing framework.
Boost Software License 1.0
257 stars 7 forks source link

Split single header and source file into separate files for development #84

Closed cschreib closed 1 year ago

cschreib commented 1 year ago

This PR does the following:

The WebAssembly Debug build in CI no longer runs Doctest, because this is triggering a bug in Node.js/V8. See below for more information.

codecov[bot] commented 1 year ago

Codecov Report

Merging #84 (1eebf1d) into main (2800a42) will decrease coverage by 0.37%. The diff coverage is 92.92%.

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/cschreib/snitch/pull/84/graphs/tree.svg?width=650&height=150&src=pr&token=X422DE81PN&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber)](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber) ```diff @@ Coverage Diff @@ ## main #84 +/- ## ========================================== - Coverage 93.29% 92.92% -0.37% ========================================== Files 2 23 +21 Lines 1237 1173 -64 ========================================== - Hits 1154 1090 -64 Misses 83 83 ``` | [Impacted Files](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber) | Coverage Δ | | |---|---|---| | [src/snitch\_error\_handling.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9lcnJvcl9oYW5kbGluZy5jcHA=) | `0.00% <0.00%> (ø)` | | | [src/snitch\_main.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9tYWluLmNwcA==) | `0.00% <0.00%> (ø)` | | | [include/snitch/snitch\_console.hpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-aW5jbHVkZS9zbml0Y2gvc25pdGNoX2NvbnNvbGUuaHBw) | `40.00% <40.00%> (ø)` | | | [src/snitch\_cli.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9jbGkuY3Bw) | `80.26% <80.26%> (ø)` | | | [src/snitch\_section.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9zZWN0aW9uLmNwcA==) | `82.35% <82.35%> (ø)` | | | [src/snitch\_test\_data.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF90ZXN0X2RhdGEuY3Bw) | `88.88% <88.88%> (ø)` | | | [src/snitch\_capture.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9jYXB0dXJlLmNwcA==) | `91.30% <91.30%> (ø)` | | | [include/snitch/snitch\_registry.hpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-aW5jbHVkZS9zbml0Y2gvc25pdGNoX3JlZ2lzdHJ5LmhwcA==) | `92.85% <92.85%> (ø)` | | | [src/snitch\_registry.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9yZWdpc3RyeS5jcHA=) | `93.11% <93.11%> (ø)` | | | [src/snitch\_append.cpp](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber#diff-c3JjL3NuaXRjaF9hcHBlbmQuY3Bw) | `94.87% <94.87%> (ø)` | | | ... and [13 more](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber) | | ... and [1 file with indirect coverage changes](https://codecov.io/gh/cschreib/snitch/pull/84/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber) ------ [Continue to review full report in Codecov by Sentry](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber). Last update [28771f7...1eebf1d](https://codecov.io/gh/cschreib/snitch/pull/84?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Corentin+Schreiber).
cschreib commented 1 year ago

The failure of the WebAssembly Debug build is, I believe, a bug in Node.js/V8. I opened a ticket there to report the issue.

Options:

  1. Wait for bug to be fixed in V8, then back-ported to Node.js some day. This could take ages, we can't realistically wait. I also don't have the skills, knowledge, and time to go fix that bug myself.
  2. Disable JIT optimisations in Node.js/V8 when running Debug builds using --liftoff-only. I'm not happy with this, because that's probably not how people are going to run the code.
  3. Disable the doctest-based tests in Debug WebAssembly builds. The snitch self-tests are running fine, but I'd be wary of not having an external testing framework cross check that we're working OK. We'd still be tested on other platforms, and in Release, so perhaps that's fine.
  4. Use another JS engine, e.g., Spidermonkey. I tried doing this, but then hit another bug in Emscripten. If I work around the bug, it seems able to run our tests, so that's promising, but I'd rather have Emscripten support this correctly from the get go. There may be other things that aren't working.
  5. Use a pure WebAssembly runtime, like wasmtime. This is almost possible but none of the pure WebAssembly runtimes support exceptions. Exception support is actually not in the WebAssembly standard; all implementations in, e.g., Emscripten and browsers are based on a proposal that hasn't been approved yet (see this post for a nice summary). So perhaps we shouldn't insist on exception support for WebAssembly for now?
cschreib commented 1 year ago

I went with option 3, since it was the lowest effort and allows me to move on. Ultimately I'd want to go for option 5, but we'll have to wait for more widespread exception support. Until then, Emscripten + Node.js seems to be the most well-supported way to run C++ WASM code.

cschreib commented 1 year ago

Ignoring the loss of coverage :)