commonize tester arguments in a single location

[spoonincode]

Change Description

When running unittests there are some optional arguments that can be given like --verbose or --(name of wasm runtime). There are some glaring problems with the current approach: the code is duplicated (actually triplicated, as I'll get to in a moment), arguments are handled at different locations, and unknown arguments are silently ignored.

The last one is a nasty gotcha. If you try running the unit tests with --wovm thinking you were going to run the unit tests with the WAVM runtime the code will silently ignore that unknown argument and run the tests with the default runtime instead. This can be incredibly deceptive.

This PR seeks the resolve all the above issues. I'm not thrilled about having to set a static member in base_tester now but it's a considerable step up from where we were IMO. There might be some other arguably better ways like moving the static variable to a global test fixture or something... if someone really wants to push for that we could do it.

🚨🚨This will break eosio.contracts because this initialization code was copy pastaed there a third time so there will be a duplicate symbol when linking with tester library. Not sure how to best handle that.

Consensus Changes

• Consensus Changes

API Changes

• API Changes

Documentation Additions

• Documentation Additions

[swatanabe-b1]

There might be some other arguably better ways like moving the static variable to a global test fixture or something... if someone really wants to push for that we could do it.

I'll push for it, because:

🚨🚨This will break eosio.contracts because this initialization code was copy pasted there a third time so there will be a duplicate symbol when linking with tester library. Not sure how to best handle that.

Moving the initialization code to a global fixture would fix this. It'll leave the initialization code running twice in cdt until cdt is updated, but I think that's harmless.

[b1bart]

I think better than a global fixture (which will be built and torn down a lot) we should use BOOST_TEST_GLOBAL_CONFIGURATION

Its not well documented but it is what Boost::Test uses to initialize its logging etc. The pro is that its guaranteed to be once per invocation.

[b1bart]

I think, if we are going for a more complete solution we should consider a few things:

is this still enabling?:

std::srand(time(NULL));
std::cout << "Random number generator seeded to " << time(NULL) << std::endl;

do we have tests that depend on std::rand and if so, can we change that? My instinct says that we don't actually consume any random numbers in our tests. If we do, that seems like something we may want to attempt to address in a way that doesn't make assumptions about the long-term stability of the sequence of random values that are produced after std::srand(time(NULL)); is called across platforms and standard library implementations.

Can we separate the needs of fc (initializing logging with the --verbose option) from the needs of the tester/chain libraries (selecting a wasm backend) without losing the ability to assert all options are known?

Related to that, is it appropriate to have a command line switch for the wasm backend selector OR is there a way to work this into the unit tests explicitly via something like BOOST_AUTO_TEST_CASE_TEMPLATE such that we can run a subset of the backends using the standard filters for test cases that the boost test framework provides?

[swatanabe-b1]

The wasm_backend is a runtime parameter, so Data driven test cases would be more appropriate than BOOST_AUTO_TEST_CASE_TEMPLATE

[b1bart]

I don't know enough about data driven testing in boost however, I suspect there will be times when people want to run a full battery of tests on only a single backend. The command line argument was nice for that and integrating it into the test cases such that you can filter by a backend would have maintained that capability.

Is there a convenient way to do that with the data driven tests?

[swatanabe-b1]

The only use of rand that I could find is in producer_scheduler_tests.cpp using boost::range::random_shuffle.

[swatanabe-b1]

I don't know enough about data driven testing in boost however, I suspect there will be times when people want to run a full battery of tests on only a single backend. The command line argument was nice for that and integrating it into the test cases such that you can filter by a backend would have maintained that capability.

Is there a convenient way to do that with the data driven tests?

You can choose tests by name, and the data is appended to the test case name, so:

test_executable --run_test='*_wavm'

[swatanabe-b1]

You can choose tests by name, and the data is appended to the test case name, so:

test_executable --run_test='*_wavm'

Never mind, that's not right.

[b1bart]

The only use of rand that I could find is in producer_scheduler_tests.cpp using boost::range::random_shuffle.

Those seem easily replaced with explicit pairwise swaps. We just want a the resulting schedule to be different than the current schedule. Each successive schedule could just ping/pong back and forth. For instance, each time std::swap(producers.front(), producers.back()); should produce a schedule thats different than the previous even if its the same as the previous's previous.

[spoonincode]

Never mind, that's not right.

I think -t '*/_wavm' would be the right syntax for that?

[swatanabe-b1]

For BOOST_DATA_TEST_CASE, the component test cases are numbered, _0, _1, _2, etc. wavm won't make it into the name, unfortunately. BOOST_AUTO_TEST_CASE_TEMPLATE does include the demangled type name, so it'll probably work better for our purposes, for now.

[spoonincode]

Nope, it seems it is not possible to "rename" the dataset's naming of each test. So it would always be named foo/_0 and foo/_1, so there would be no way to enable/disable all test for a runtime via a name that way. We could make the dataset dynamically change its indexes based on the command line.

[swatanabe-b1]

The only use of rand that I could find is in producer_scheduler_tests.cpp using boost::range::random_shuffle.

Those seem easily replaced with explicit pairwise swaps. We just want a the resulting schedule to be different than the current schedule. Each successive schedule could just ping/pong back and forth. For instance, each time std::swap(producers.front(), producers.back()); should produce a schedule thats different than the previous even if its the same as the previous's previous.

How about std::next_permutation?

[b1bart]

How about std::next_permutation?

I don't have any major issue with it. Minor issue is it seems like overkill but I agree that the intent of the code is more legible than the std::swap version.

[spoonincode]

seems like BOOST_AUTO_TEST_CASE_TEMPLATE loses fixture support, is that something we're okay with?

[b1bart]

seems like BOOST_AUTO_TEST_CASE_TEMPLATE loses fixture support, is that something we're okay with?

In a world with no perfect solutions, I think I'd prefer to keep fixture support and solve this through one of the other options. If that means we are still parsing "extra" options after the standard boost options with some global or fixture or configuration then so be it.

[swatanabe-b1]

BOOST_FIXTURE_TEST_CASE_TEMPLATE exists, even though it isn't documented. BOOST_FIXTURE_TEST_SUITE will also work and is documented. Alternately, Boost.Test exposes sufficient APIs to write our own macros that can adjust the test case name any way we want.

[spoonincode]

considering that it was added 10 years ago it probably just is sloppy documentation then

[spoonincode]

The template mechanism looked really promising, something like:

BOOST_FIXTURE_TEST_CASE_TEMPLATE( my_test, T, wasm_runtimes, tester<T> )
{
  dothedew();
}

but unfortunately this doesn't work because of dependent name lookups. Has to be more like

BOOST_FIXTURE_TEST_CASE_TEMPLATE( my_test, T, wasm_runtimes, tester<T> )
{
  this->dothedew();
}

which of course drastically reduces the usefulness of the fixture. So unfortunately it seems like we will have make something more custom

[swatanabe-b1]

I put together a prototype of a custom test case a few days ago.
boost-test-extensions.tar.gz