Sounds impressive.

Here are the important definitions (this PRs diff): https://github.com/bitcoin/bitcoin/pull/8650/files#diff-4a0cb5ad5e93d187a1b065a99bbae143R17

Documentation: http://www.boost.org/doc/libs/1_34_0/libs/test/doc/components/test_tools/reference/BOOST_CHECK_MESSAGE.html

Concept ACK

Concept ACK. After:

git diff HEAD~1 --name-only | xargs sed -i "s/FAST_CHECK/BOOST_CHECK/g"

There is a remaining diff in test_bitcoin.h:

+#define BOOST_CHECK(x) if (!(x)) { BOOST_CHECK_MESSAGE(false, #x); }
+#define BOOST_CHECK_EQUAL(x, y) if ((x)!=(y)) { BOOST_CHECK_MESSAGE(false, "(" #x ") != (" #y ")"); }
+#define BOOST_CHECK_THROW(expr, ex) try { expr; BOOST_CHECK_MESSAGE(false, "( " #expr " ) did not throw ( " #ex " )"); } catch( ex ) { }
+#define BOOST_CHECK_NO_THROW(expr) try { expr;  } catch( ... ) {BOOST_CHECK_MESSAGE(false, "( " #expr " ) threw exception"); }
+#define BOOST_CHECK_EXCEPTION(expr, ex, pred) try { expr; BOOST_CHECK_MESSAGE(false, "( " #expr " ) did not throw ( " #ex " )"); } catch( ex& a ) { if (!pred(a)) BOOST_CHECK_MESSAGE(false, "( " #expr " ) did not throw ( " #ex " ) under ( " #pred " )" ); }
+#define BOOST_CHECK_EQUAL_COLLECTIONS(l, r, l1, r1) BOOST_CHECK_EQUAL_COLLECTIONS(l, r, l1, r1) 
+

Any specific reason for (re-)defining these?

Oh never mind - these are replaced of course @jonasschnelli has the right definitions. So you define custom testing macros, right.

Q: What functionality do we lose here, apart from being able to log successful tests (which is arguably unnecessary)?
For example I vaguely remember that the BOOST_CHECK_EQUAL does an attempt to print the values, not just the stringified form of the input arguments.

At some point we likely want to get rid of boost::test completely. as we are trying to eventually remove the dependency on boost, so this is a step in the right direction.

@laanwj I may be able to make it completely compatible minus the succeeding tests for each macro. You are correct that boost_check_equal does that; this is part of why I did not redefine the container equal one.

What I'm unsure of is how that affects licensing given those would be a derivative work I believe -- perhaps I could break out my macros into a sub-library and keep it boost licensed?

Trivial rebase required. Also, I was wondering how we could forbid usage of the "legacy" `BOOST_CHECK_* to prevent diverging code.

perhaps I could break out my macros into a sub-library and keep it boost licensed?

Right, that makes sense: if you're using any boost code then it's a derivative work, if you're just using the names of the macros for compatibility it's not. Do note that the boost license is more permissive than MIT/BSD (no attribution requirement), so including a boost-licensed source file is not a problem.

Huh? I didn't see any boost source code added, so the current pull should be good license wise.

Yes the current pull is, but (read back a few posts) he is talking about extending the functionality to something completely compatible.

@laanwj I don't see how we lose functionality.

@ Jeremy Check your mail. It should be trivial to keep the log messages the
same as well

On Friday, September 2, 2016, Wladimir J. van der Laan <
[email protected]> wrote:

Q: What functionality do we lose here, apart from being able to log
successful tests (which is arguably unnecessary)?
For example I vaguely remember that the BOOST_CHECK_EQUAL does an attempt
to print the values, not just the stringified form of the input arguments.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.<
https://ci5.googleusercontent.com/proxy/y-3C4A3Q1wrYbmH2Ylkt2A0H5t_g18JH93W1j4X-rMvxva57umGu8LYbGWrx-2KG43iGXvTJzOYsJCFPwGAeUUoshU8yjvuwI0DUJmPFos2gj90HbuEBbw1ji_Lzy2RjVD8YeGwUXfgyHCv6q1DB9eGtazfHQQ=s0-d-e1-ft#https://github.com/notifications/beacon/AGGmv0llHp5MBym3vyMXyqBWvwnwX5n9ks5ql-RjgaJpZM4JzXok.gif>

@laanwj I don't see how we lose functionality.

That was my question. One thing I noticed was that BOOST_EQUAL no longer prints the contents of what it compares but just stringified variable names (which is much less useful). But I was not sure either if there is more.

@MarcoFalke @laanwj

A few notes:

1. BOOST_CHECK and friends always format the string. This is why part of why it is slow.
2. We can't use Marco's compatibility fix that he emailed me because it is macro-unhygenic
3. I could just rip the boost macros and replace them with a patched, lighter weight version.
4. I could just entirely replace boost unit testing and we can not care about preserving functionality completely.

I think that 4. is probably the best option longer term.

Concept ACK.

As far as lost functionality, note that we also lose the checkpoints this way, so if a segfault is introduced, it's possible that it's harder to tell where it's coming from. I suspect that's part of what makes the macros so expensive to begin with.

I don't think those are a huge loss, they've never helped me much.

Ok, here's what I'm going to do. I've just opened up #8671 which does a very minimal change to prevector tests that should fix the underlying issue; while giving more useful debugging output (the rand seeds needed to recreate this test case). I think there's no reason this can't merge almost immediately as a much needed stopgap to make the tests just run faster.

In the meantime, I've started working on a boost-free test suite and opened an issue about it #8670. All necessary features should go in there, such as checkpoints being needed by @sipa.

(@MarcoFalke @laanwj @theuni @jonasschnelli )