-
Notifications
You must be signed in to change notification settings - Fork 236
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
Consumed catch2 from usersim path #3973
base: main
Are you sure you want to change the base?
Consumed catch2 from usersim path #3973
Conversation
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.
Looks good to me other than needing one line deleted.
Thanks for doing this!
2505004
to
b58bef7
Compare
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.
You still need to delete the original catch2 line from dependabot.yml
b2ac67d
to
9f297ef
Compare
18efeef
to
84a7751
Compare
@dthaler few tests are failing for this build, This error is ambiguous for me, is it related catch2 registration failure as the path in catch_wrapper file is not correct? or is it something else? Would appreciate your help. |
I don't know, but it seems to be specific to netebpfext. @Alan-Jowett @shankarseal have you seen this before? |
Can you refer to the stack dump here: #3972 (comment) |
Looking at the stack trace, catch2 is still being searched at the previous path for netebpfext_unit in here : netebpfext_unit!Catch::operator<<+0x5d [D:\a\ebpf-for-windows\ebpf-for-windows\external\Catch2\src\catch2\internal\catch_lazy_expr.cpp @ 20] so, do I need to update path in catch_wrapper.hpp? as I am not sure from where is the above path picked up |
@agarwalishita I looked at the crash dump of netebpf_unit_tests from this PR.
Code in netebpfext_unit.cpp
Build logs:
|
Since the test cases were passing in the baseline, I would like you to check the below:
To rule out, can you check the versions of the Catch2 in usersim repo to compare against? (Maybe, type_traits assert can be caused by mismatch of msvc tools version). Also, netebpfext_unit.cpp has included: Hence check the test\libs\util\catch_wrapper.hpp for path and correctness |
@shpalani These have the same versions for me as well Regarding catch_wrapper, as previously mentioned, I am unsure how the library is able to fetch the Catch2 path from an external source directly without specifying its path. If I need to make a change here, what path should I use? |
The path seems right in tests/libs/util/test_util.vcxproj. From your PR code, where the path is being included for catch_all.hpp is |
Description
Catch2 was getting synced at 2 paths:
external\catch2
external\usersim\external\catch2
So, the former submodule has been deleted as part of this PR
and the source path used is : external\usersim\external\catch2 for creating build file