Skip to content
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 emscripten automated tests #483

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mcbarton
Copy link
Collaborator

@mcbarton mcbarton commented Jan 24, 2025

Description

Please include a summary of changes, motivation and context for this PR.

My first PR to add automated emscripten tests became broken for unknown reasons (see #448). This PR is another attempt to added emscripten tests to CppInterOp

Fixes # (issue)

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

  • I have read the contribution guide recently

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.17%. Comparing base (b797dbb) to head (f1d994e).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #483   +/-   ##
=======================================
  Coverage   71.17%   71.17%           
=======================================
  Files           9        9           
  Lines        3552     3552           
=======================================
  Hits         2528     2528           
  Misses       1024     1024           

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

unittests/CppInterOp/InterpreterTest.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

unittests/CppInterOp/InterpreterTest.cpp Outdated Show resolved Hide resolved
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch 2 times, most recently from f3d8b34 to e211109 Compare January 26, 2025 12:50
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

unittests/CppInterOp/Utils.h Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

unittests/CppInterOp/main.cpp Show resolved Hide resolved
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch 5 times, most recently from 027e9a3 to ab5976a Compare February 9, 2025 16:14
@mcbarton mcbarton changed the title [wip] Add emscripten automated tests Add emscripten automated tests Feb 9, 2025
@mcbarton mcbarton marked this pull request as ready for review February 9, 2025 16:15
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch from bba850e to 123e15c Compare February 9, 2025 16:27
Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am adding a few comments from my partial review.

CMakeLists.txt Outdated Show resolved Hide resolved
cmake/modules/GoogleTest.cmake Outdated Show resolved Hide resolved
cmake/modules/GoogleTest.cmake Show resolved Hide resolved
lib/Interpreter/CMakeLists.txt Outdated Show resolved Hide resolved
unittests/CppInterOp/CMakeLists.txt Outdated Show resolved Hide resolved
unittests/CppInterOp/main.cpp Show resolved Hide resolved
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch from 8ec905a to 827b696 Compare February 9, 2025 20:10
@mcbarton mcbarton requested a review from vgvassilev February 9, 2025 20:30
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch 3 times, most recently from e2521fe to b70047b Compare February 12, 2025 13:49
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch 2 times, most recently from d282fe4 to 092eddd Compare February 13, 2025 09:29
@mcbarton mcbarton requested a review from vgvassilev February 13, 2025 10:12
Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anutosh491, can you take a look?

unittests/CppInterOp/TestSharedLib/CMakeLists.txt Outdated Show resolved Hide resolved
unittests/CppInterOp/TestSharedLib/CMakeLists.txt Outdated Show resolved Hide resolved
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch 2 times, most recently from 24141c0 to 9f02e74 Compare February 13, 2025 16:27
@mcbarton mcbarton force-pushed the wip]-add-automated-wasm-tests branch from 479c8a2 to f1d994e Compare February 13, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants