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 basic thread safety #117

Closed
wants to merge 2 commits into from
Closed

Conversation

blastrock
Copy link

Description

I added basic thread-safety to doctest to be able to do assertions in multiple threads. It only works with C++11 support though.

I ran some tests with clang's thread sanitizer to make sure there are no more races.

GitHub Issues

Closes #4

@blastrock blastrock changed the title Threadsafe Add basic thread safety Feb 7, 2018
@onqtam
Copy link
Member

onqtam commented Feb 7, 2018

Hello and thanks for the effort - I see the original issue was opened by you so this must be pretty important!

Thanks for this baseline - I'm gonna take a look at it in the next couple of weeks (hopefully) - I'm still knee-deep with other work and cannot commit a few days/weeks for a new version.

I was planning on refactoring most of the internals of the framework in order to add support for reporters and then to look at thread safety, but that order may change.

I won't merge it instantly, but this will most likely go into version 1.3 :)

@blastrock
Copy link
Author

No problem :)

Reporters may be hard to do threadsafe, but I think you can just mutex them for a start and see later how to avoid locks. I may be able to help there :)

@onqtam onqtam closed this Mar 10, 2018
@onqtam
Copy link
Member

onqtam commented Apr 9, 2018

not sure why I closed this - It will go in (one way or another) - but I'll have to postpone it with at least 1 more month...

@onqtam onqtam reopened this Apr 9, 2018
@onqtam
Copy link
Member

onqtam commented May 8, 2018

I did some refactoring before merging this. Now it has conflicts. I will either try to resolve them or re-do the changes again. I was also planning on changing it a bit - sorry if I don't end up merging this (but its a good starting point!) - but at least thread safety is coming in 1.3 which I plan on releasing in the next month :)

@blastrock
Copy link
Author

No problem, I'm more interested in getting the feature than in merging this ;)

@onqtam onqtam closed this May 10, 2018
@onqtam
Copy link
Member

onqtam commented May 10, 2018

apparently deleting the dev branch resulted in closing this PR. I'm publishing version 1.2.9 now and in the next weeks will be working on 1.3 with this (in some form) in it!

@onqtam onqtam reopened this May 10, 2018
@onqtam
Copy link
Member

onqtam commented May 30, 2018

FYI I'll soon be working on this - I've rewritten the guts of doctest and have moved to streams for the reporting - I've also made a reporter interface which can be used by users to register alternative reporters (no documentation yet).

I also decided to switch to C++11 features and doctest is currently 5100 lines of code compared to 5700 for version 1.2.8 - lots of code got removed, but sadly GCC 4.4, 4.5 and 4.6 will no longer be supported (also Visual Studio 2008/2010/2012).....

At least now I can straight up include <thread> - no more conditional compilation!

So this will get into version 2.0 but still I cannot promise a date - hopefully within the next month... This is going to be the biggest release since 1.0 !

onqtam added a commit that referenced this pull request Jul 11, 2018
…est case is now safe! using the SUBCASE macro in multiple threads will remain a big NO. TODO: logging macros such as INFO() and CAPTURE()... relates #4 and also PR #117 which was a good starting point for this!
@onqtam
Copy link
Member

onqtam commented Jul 11, 2018

I just pushed the thread safety support in commit 60e8a45 in the dev branch! TODO: the logging facilities. I'm closing this but thanks again for this PR and showing me how easy this was!

I will not promise a release date however... but the work I have left for version 2.0 isn't much - all I have to do is implement an xml reporter with the new IReporter interface - but I should research xml encoding and other non-interesting things... :D

@blastrock
Copy link
Author

Great! Thank you! And good luck with XML :)

onqtam added a commit that referenced this pull request Aug 23, 2018
…est case is now safe! using the SUBCASE macro in multiple threads will remain a big NO. TODO: logging macros such as INFO() and CAPTURE()... relates #4 and also PR #117 which was a good starting point for this!
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