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.

Sign up for GitHub

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

Jump to bottom

Add portable uBPF unit tests that run on Windows / Linux / MacOS #119

Merged
merged 7 commits into from
Oct 13, 2022
Merged

Add portable uBPF unit tests that run on Windows / Linux / MacOS #119

merged 7 commits into from
Oct 13, 2022

Conversation

Alan-Jowett
Copy link
Collaborator

@Alan-Jowett Alan-Jowett commented Sep 30, 2022 •
edited
Loading

Run the uBPF code generation tests and capture code coverage..

Resolves: #117
Resolves: #141
Resolves: #149

Signed-off-by: Alan Jowett alan.jowett@microsoft.com

@Alan-Jowett Alan-Jowett force-pushed the add_code_gen_tests branch 11 times, most recently from c8689df to cd2f3f2 Compare September 30, 2022 18:18
@Alan-Jowett Alan-Jowett marked this pull request as ready for review September 30, 2022 18:27
@Alan-Jowett Alan-Jowett changed the title uBPF unit tests that run natively with out Python. Add portable uBPF unit tests that run on Windows / Linux / MacOS Sep 30, 2022
@Alan-Jowett Alan-Jowett force-pushed the add_code_gen_tests branch 5 times, most recently from fd103ff to 954756f Compare September 30, 2022 23:44
@Alan-Jowett
Copy link
Collaborator Author

@alessandrogario this is a continuation of your work to have uBPF build on Windows, Linux, and MacOS. Would you be able to take a quick look at this?

@Alan-Jowett
Copy link
Collaborator Author

@rlane can you authorize the coveralls.io app so it can post code coverage stats for PRs?

@Alan-Jowett Alan-Jowett mentioned this pull request Oct 3, 2022
Closed
@matt-gretton-dann
Copy link
Collaborator

I have tested this on an M1 Mac and under Arm64 Linux with no issues.

Thank you - this fixes many Python version contortions I needed to do.

@Alan-Jowett
Copy link
Collaborator Author

Awesome. I hope to eventually retire the Python-based tests, but there are still some code coverage gaps in the C++ tests that need to be addressed. I want to get this checked in as the first step towards getting this updated.

@Alan-Jowett Alan-Jowett force-pushed the add_code_gen_tests branch 5 times, most recently from 9e01d7d to 86f5276 Compare October 4, 2022 20:28
@Alan-Jowett
Copy link
Collaborator Author

Need to refactor this now that the parsing and testing code has been lifted into it's own repo bpf-conformance.

@Alan-Jowett Alan-Jowett force-pushed the add_code_gen_tests branch 2 times, most recently from 14532a1 to 25dc1e1 Compare October 8, 2022 19:59
@Alan-Jowett
Copy link
Collaborator Author

@matt-gretton-dann can you check on your MacOS and see what the correct package name for the equivalent of "libboost-dev", "libboost-program-options-dev", and "libboost-filesystem-dev" are. I think "boost" is the correct for "libboost-dev", but I don't have access to a MacOS instance to check the others.

@Alan-Jowett Alan-Jowett marked this pull request as draft October 9, 2022 20:13
@Alan-Jowett
Copy link
Collaborator Author

Converting to draft until MacOS and ARM64 issues are resolved.

@matt-gretton-dann
Copy link
Collaborator

@matt-gretton-dann can you check on your MacOS and see what the correct package name for the equivalent of "libboost-dev", "libboost-program-options-dev", and "libboost-filesystem-dev" are. I think "boost" is the correct for "libboost-dev", but I don't have access to a MacOS instance to check the others.

@Alan-Jowett: boost is all you need. However, there are problems in the CMakeLists.txt file, and also in bpf_conformance that need resolving.

@matt-gretton-dann
Copy link
Collaborator

@Alan-Jowett: To help fix the macOS build Alan-Jowett/bpf_conformance#24 needs to be approved, and then the submodule dependency updated to the appropriate hash.

matt-gretton-dann
matt-gretton-dann reviewed Oct 10, 2022
View reviewed changes
unit_tests/CMakeLists.txt Outdated Show resolved Hide resolved
.github/workflows/posix.yml Outdated Show resolved Hide resolved
@Alan-Jowett
Copy link
Collaborator Author

Thanks for the help @matt-gretton-dann. The MacOS ones now pass. Hopefully I will get the amr64 fixed next.

@Alan-Jowett
Add tests using bpf_conformance library
521675d 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett
Copy link
Collaborator Author

Everything appears to build and run.
Windows Debug/Release
Linux x64 Debug/Release | Sanitize/Coverage
Linux arm64 Debug/Release | Sanitize/Coverage
MacOS11 Debug/Release | Sanitize/Coverage

@Alan-Jowett
Add arm64 sanitizer pass
3b48c7d 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett Alan-Jowett force-pushed the add_code_gen_tests branch 3 times, most recently from b0d0fee to 003a47b Compare October 12, 2022 17:43
@Alan-Jowett
Limit macOS to aligned load/store
c33cc8b 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@matt-gretton-dann matt-gretton-dann mentioned this pull request Oct 13, 2022
Merged
@Alan-Jowett
Merge branch 'main' into add_code_gen_tests
39f7c0a
@Alan-Jowett
Add option to debug ASAN failures
9884b6e 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett
Disable ubsan until we can resolve the issues
bf7892b 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett
Copy link
Collaborator Author

There is value in getting this checked in. Filed an issue to resolve the ubsan / qemu issue later.

@Alan-Jowett Alan-Jowett marked this pull request as ready for review October 13, 2022 15:46
@Alan-Jowett
Remove dead code
c5ab23f 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
matt-gretton-dann
matt-gretton-dann approved these changes Oct 13, 2022
View reviewed changes
Copy link
Collaborator

@matt-gretton-dann matt-gretton-dann left a comment

Choose a reason for hiding this comment

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

This now looks good to me and has great value in improving quality of life when building & testing, so any further issues can be fixed in future PRs.

@matt-gretton-dann matt-gretton-dann merged commit 6bd288b into iovisor:main Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matt-gretton-dann matt-gretton-dann matt-gretton-dann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@Alan-Jowett @matt-gretton-dann