Port to C++11/C++14 removing superflous Boost facilities #88

Flamefire · 2020-07-12T11:46:35Z

This addresses #69 by replacing Boost components for which equivalents or improved versions in C++11/14 exist.

This will reduce compile times for real world projects and potentially make test easier to debug.

coveralls · 2020-07-13T14:55:51Z

Coverage decreased (-0.02%) to 97.8% when pulling 41148ea on Flamefire:c++11 into bfd1701 on mat007:master.

Flamefire · 2020-07-15T10:29:47Z

@mat007 I'm done for the first step, appveyor failure is doc generation related and just a HTTP error.

I'll rebase once #90 is merged to make this a bit smaller but you can have a look already. Next step would be replacing Boost.PP usage by variadic templates which should make the code much more understandable. If wanted I can try to port the inclusion of the benchmarks into the build to current master so we can compare build times before/after for those. Note that those are not fully accurate as one can assume the std headers are used already while the benchmarks assume a "clean state"

Flamefire · 2020-07-16T12:15:53Z

@mat007 Ready. The decreased coverage is due to https://coveralls.io/builds/32087216/source?filename=include/turtle/detail/expectation_template.hpp where I reduced the number of lines by 7 which causes a decrease in relative coverage. Might be worth checking if we can test the currently uncovered serialization functions.

mat007

Awesome work on the code makeover, thanks for doing this!

I’m not totally convinced on the testing of the documentation code though, I understand the value but this adds many visible side effects for the users to wonder why they’re in the docs.
Can’t we split these out to a different PR so that we can merge the rest?

doc/example/calculator.hpp

doc/example/customization.cpp

mat007 · 2020-08-03T06:03:37Z

doc/example/getting_started.cpp

 {
   mock_view v;
   calculator c( v );
   MOCK_EXPECT( v.display ).once().with( 0 ); // missing returns( true )
   c.add( 0, 0 );
+   assert_failure("missing action");


This is showing up in the docs, I don’t think this was intentional, was it?

It is. The docs say that at this point a test assertion fails that an action is missing. This highlights it AND actually tests it

I’m thinking some newcomers will be puzzled by this line, especially since the definition for assert_failure will not be visible.
There should maybe at least be a comment?

If we upgrade the required boost version to 1.59 (or higher) we can use assertions in fixtures teardown

Initial support via BOOST_AUTO_TEST_CASE(test, *utf::fixture<MixFixture>) is in 1.59

Better in 1.65 via BOOST_FIXTURE_TEST_CASE(test, MyFixture) (like now but teardown() is supported in 1.65+)

What shall it be?

I think 1.65 is fine if that helps.

Upgraded that for new CI

doc/example/getting_started.cpp

include/turtle/detail/type_name.hpp

include/turtle/detail/void_t.hpp

include/turtle/mock.hpp

test/detail/test_type_name.cpp

test/test_log.cpp

Flamefire · 2020-08-03T12:15:14Z

As many comments are for code style: would you mind creating a clangformat file for clangformat 9 which I could use? I find it pretty hard to guess how to do that otherwise... Or if you don't care which style is used I could use one from another project. Avoids the bikeshedding and confusion...

About the docs: sure I can extract that but this would be hard as I had to fix them and used the new changes already there. So they would need to be merged afterwards. But then I feel that test coverage is missing. Some macros are only used there and I might miss a bug.
Maybe leave them in here until the pr is ready and I extract them to a follow up just before merging if you really want them separately.

I'll address the other comments next week when I work on this again. A style guide or definition until then would be nice. Check https://github.com/Return-To-The-Roots or https://github.com/boostorg/nowide for example configs which would be easiest

Flamefire · 2020-08-12T15:39:07Z

I rebased this to current master to resolve the conflict.

@mat007 I'm still waiting for your call on using clang-format which would greatly reduce the work for keeping the formatting uniform.

Flamefire · 2020-09-01T08:13:55Z

@mat007 Kindly requesting on guidance how to continue on this. If wanted we could meet in a chat or so if there is stuff for discussion so it is faster than through github. I'd like to build more stuff on top of this like replacing the preprocessor generated implementations by variadic templates to make it easier to read and debug and using a macro that allows method specifiers (const, ref qualifiers) and especially override support. This will make the mock much safer and avoids many warnings we have in our code.

mat007 · 2020-09-05T06:40:05Z

@mat007 Kindly requesting on guidance how to continue on this.

Sorry, I’ve been away quite a lot over the summer.
I think they were two remaining points that we need solving: formatting and docs.

As many comments are for code style: would you mind creating a clangformat file for clangformat 9 which I could use? I find it pretty hard to guess how to do that otherwise... Or if you don't care which style is used I could use one from another project. Avoids the bikeshedding and confusion...

I don’t care as long as it’s used everywhere in the code.
Doesn’t Boost suggest a clangformat file? Or some of the library authors?

About the docs: sure I can extract that but this would be hard as I had to fix them and used the new changes already there. So they would need to be merged afterwards. But then I feel that test coverage is missing. Some macros are only used there and I might miss a bug.
Maybe leave them in here until the pr is ready and I extract them to a follow up just before merging if you really want them separately.

I liked how the reference documentation only explained each bit one at a time, but I can understand how making sure it compiles helps.
I’ll take a look to see what it looks like now, but I don’t think it’s a blocker for the PR as it can be adjusted later if needed.

Again, sorry for my poor reactivity this last couple of months!

Tests via Fixture::teardown that the assertion is fulfilled

…r_passed_as_parameter_of_a_mock_method Only show what is required

I.e. it means the maximum allowed difference, similar to how other testing frameworks handle this and it allows a threshold of 0 to mean "equal"

This is allowed in C++20 and doesn't need to be tested anyway.

This avoids false positives when warnings-as-error is enabled

- Move travis to GHA (travis is dead for OSS) - Update Boost on appveyor

Flamefire · 2022-01-07T22:29:31Z

@mat007 After some heavy fighting against the CI and some peculiarities of the Ubuntu linker I finally managed to get this working on CI again including the coverage report.
I addressed your review comments unfortunately the history got a bit messed up. Basically the last 8 commits starting at 728dfd0 are new incorporating your requests and the new CI. The tests now also pass on C++20.

After this is merged I can quickly add the formatting as discussed in #97

jonesmz · 2022-01-10T18:42:49Z

include/turtle/detail/parameter.hpp

    };
+    template<typename T>
+    using parameter_type_t = typename parameter_type<T>::type;


Would it make sense to use boost::callable_traits for parameter type deduction instead?

Would be possible. But similar to is_callable this only supports simple methods which seemingly is(/was?) enough, so this (much smaller) implementation should suffice.
And Boost.CallableTraits was introduced in Boost 1.66, i.e. it would need another increase in the minimum Boost version required which I wanted to avoid at the time of writing this.

jonesmz · 2022-01-10T18:47:45Z

include/turtle/log.hpp

+    {};
+    template<typename T>
+    struct is_callable: is_callable_impl< std::remove_cv_t<T> >
+    {};


https://www.boost.org/doc/libs/master/libs/callable_traits/doc/html/callable_traits/reference.html#callable_traits.reference.ref_is_invocable ?

Not really. I tried to translate what was there before and this whole thing isn't really good anyway: Besides a plain function there isn't a Type "callable" (this trait only takes a single parameter!)
Actually you would need to check if you can invoke something with specific parameters as is_invocable does. But we don't have those.

So a better name would be e.g. is_function.

mat007

Thanks @Flamefire this is a huge achievement, and I have to tip my hat off to you for your commitment! 😁

I can either merge the PR, or I can add you as a collaborator to the project and you can merge it yourself? (you don’t have to accept if you don’t feel like it, no worries)

mat007 · 2022-01-23T14:01:16Z

doc/example/limitations_template_method.cpp

+{
+    mock_concept b;
+    MOCK_EXPECT(b.method_int).once().with(42);
+    MOCK_EXPECT(b.method_string).once().with(mock::equal(std::string("string")));


I still think mock::equal<std::string> is a bit clearer, as in meaning «equal as a string», but that’s pretty minor.

Flamefire · 2022-01-23T14:20:25Z

I can either merge the PR, or I can add you as a collaborator to the project and you can merge it yourself? (you don’t have to accept if you don’t feel like it, no worries)

Either is fine for me ;)
Next thing I'll open a PR for is the formatting change as previewed in #97

Flamefire marked this pull request as draft July 12, 2020 11:46

Flamefire force-pushed the c++11 branch 8 times, most recently from 937c321 to ec41d7c Compare July 13, 2020 14:05

Flamefire force-pushed the c++11 branch 8 times, most recently from 9e9b5ed to f51dce4 Compare July 14, 2020 19:23

Flamefire force-pushed the c++11 branch from f51dce4 to 6f6836f Compare July 15, 2020 14:51

Flamefire marked this pull request as ready for review July 16, 2020 12:14

mat007 reviewed Aug 3, 2020

View reviewed changes

Flamefire force-pushed the c++11 branch from 6f6836f to 0bb9f42 Compare August 12, 2020 15:37

Flamefire force-pushed the c++11 branch from 0bb9f42 to 4771a4d Compare September 1, 2020 08:10

Flamefire mentioned this pull request Sep 5, 2020

Preview of clang-format changes and CI #97

Closed

Flamefire added 7 commits January 7, 2022 17:08

Add documentation about unwrap_ref back and add example

40a4b3c

Remove user(doc) visible change to zero_plus_zero_is_zero_with_action

728dfd0

Tests via Fixture::teardown that the assertion is fulfilled

Remove test-artifact from user visible part of how_to_invoke_a_functo…

e430594

…r_passed_as_parameter_of_a_mock_method Only show what is required

Remove ASSERT_VERIFY_FAIL macro from static_objects_problem

e266016

Change the threshold param of near to be inclusive

98541eb

I.e. it means the maximum allowed difference, similar to how other testing frameworks handle this and it allows a threshold of 0 to mean "equal"

Remove test checking for failure when NOT using "typename"/TPL macro

78fa67e

This is allowed in C++20 and doesn't need to be tested anyway.

Rename class concept to avoid name clash with C++20 concept keyword

f4c7401

Flamefire force-pushed the c++11 branch 5 times, most recently from 8ec89dc to 182e070 Compare January 7, 2022 17:13

Avoid unused function warnings in compile fail tests

2dede83

This avoids false positives when warnings-as-error is enabled

Flamefire force-pushed the c++11 branch 6 times, most recently from 6bce285 to 9551dea Compare January 7, 2022 21:47

Update CI

23ed958

- Move travis to GHA (travis is dead for OSS) - Update Boost on appveyor

Flamefire force-pushed the c++11 branch from 9551dea to 23ed958 Compare January 7, 2022 22:17

jonesmz reviewed Jan 10, 2022

View reviewed changes

mat007 approved these changes Jan 23, 2022

View reviewed changes

Flamefire merged commit b5bb500 into mat007:master Jan 23, 2022

This was referenced Jan 24, 2022

deprecation message with boost 1.74.0 #98

Closed

Corrected boost bind deprecation warning #104

Closed

Flamefire mentioned this pull request Feb 3, 2022

Replace Boost PP_Iterate by C++11 variadic templates #110

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to C++11/C++14 removing superflous Boost facilities #88

Port to C++11/C++14 removing superflous Boost facilities #88

Flamefire commented Jul 12, 2020 •

edited

Loading

coveralls commented Jul 13, 2020 •

edited

Loading

Flamefire commented Jul 15, 2020

Flamefire commented Jul 16, 2020

mat007 left a comment

mat007 Aug 3, 2020

Flamefire Aug 3, 2020

mat007 Sep 20, 2020

Flamefire Sep 24, 2020

mat007 Oct 22, 2020

Flamefire Jan 6, 2022

Flamefire commented Aug 3, 2020

Flamefire commented Aug 12, 2020

Flamefire commented Sep 1, 2020

mat007 commented Sep 5, 2020

Flamefire commented Jan 7, 2022 •

edited

Loading

jonesmz Jan 10, 2022

Flamefire Jan 12, 2022

jonesmz Jan 10, 2022

Flamefire Jan 12, 2022

mat007 left a comment •

edited

Loading

mat007 Jan 23, 2022

Flamefire commented Jan 23, 2022

Port to C++11/C++14 removing superflous Boost facilities #88

Port to C++11/C++14 removing superflous Boost facilities #88

Conversation

Flamefire commented Jul 12, 2020 • edited Loading

coveralls commented Jul 13, 2020 • edited Loading

Flamefire commented Jul 15, 2020

Flamefire commented Jul 16, 2020

mat007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Flamefire commented Aug 3, 2020

Flamefire commented Aug 12, 2020

Flamefire commented Sep 1, 2020

mat007 commented Sep 5, 2020

Flamefire commented Jan 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mat007 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Flamefire commented Jan 23, 2022

Flamefire commented Jul 12, 2020 •

edited

Loading

coveralls commented Jul 13, 2020 •

edited

Loading

Flamefire commented Jan 7, 2022 •

edited

Loading

mat007 left a comment •

edited

Loading