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

test discovery #5

Merged

Conversation

Garcia6l20
Copy link

  • adding doctest discovery (doctest.cmake)

- adding doctest discovery (doctest.cmake)
@Garcia6l20
Copy link
Author

This is a cosmetic PR but results in nicer test reports

@andreasbuhr
Copy link
Owner

Wow, this is really great! I merged "integrate_all" and your proposed change and pushed it as branch "deleteme2" to see what github actions do. In all configurations, the test "multi-threaded" runs into a timeout, see https://github.com/andreasbuhr/cppcoro/runs/1248616370 . Did you also observe this? Is this easy to fix? If not, can we disable the test until we have a fix?

@Garcia6l20
Copy link
Author

I've been lucky: https://github.com/Garcia6l20/cppcoro/runs/1252808292?check_suite_focus=true
But this test took 23.7 sec, so maybe we should set an higher timeout !

@Garcia6l20
Copy link
Author

Do you observe this behavior on you laptop ?
I dont, so I think it might be related to BH's VMs, don't you think so ?

@Garcia6l20
Copy link
Author

Notice we have 2 multi-threaded test,
TEST_SUITE are not (yet) handled by doctest_discover_tests

doctest/doctest#302

@andreasbuhr
Copy link
Owner

Increasing the timeout for the one test which sometimes runs into timeout sound like a good solution to me. Would you do it?

@Garcia6l20
Copy link
Author

Yes,
I also added sanitizers in my io_service branch, it would be really helpful for everyone, but we will also have to update doctest cause the version we are using have some problems with that, and it looks like it have been resolved on current master branch.

@andreasbuhr
Copy link
Owner

Hi, oh yes, sanitizers would be cool.
Just that I understand it correctly: An update to doctest version 2.4.0 as I prepared in lewissbaker#178 would not be sufficient? Does it have to be current master branch?

@Garcia6l20
Copy link
Author

Idk, I just know they are fixed now I'll try out this version after.

@Garcia6l20
Copy link
Author

@Garcia6l20
Copy link
Author

Well it work... hum... it fails but it works :)
I forced a leak cause we cppcoro looks leak-free :)

https://github.com/Garcia6l20/cppcoro/runs/1255719412

ubsan seems not working with coroutines
and we'll have to investigate with tsan

@andreasbuhr
Copy link
Owner

This looks really great, thank you so much.

@andreasbuhr andreasbuhr merged commit dfeca19 into andreasbuhr:add_cmake_support Oct 14, 2020
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