Test Fix: Controller calls only valid when building a block #1442
Conversation
tests/test_read_only_trx.cpp
Outdated
| } ); | ||
| fc::scoped_exit<std::function<void()>> on_except = [&](){ | ||
| if (app_thread.joinable()) |
There was a problem hiding this comment.
Instread of having these two app_thread.join();, why not put line 102 to 176 in a block and rely on the scoped_exit to join the app_thread?
| if (trx_future.wait_for(std::chrono::seconds(5)) == std::future_status::timeout) | ||
| throw std::runtime_error("failed to execute trx: " + ptrx->get_transaction().actions.at(0).name.to_string() + " to account: " + account.to_string()); |
There was a problem hiding this comment.
So we posted on the app_thread a lambda containing references to local variables. And, upon a 5s timer, we can throw an exception and exit the stack frame holding these local variable. I Think it could be an issue if the app thread executes the lambda afterwards.
Maybe it would be better to make_shared the promise and pass a shared_ptr to it?
unittests/test_utils.hpp
Outdated
| std::atomic<bool> feature_set = false; | ||
| // has to execute when pending block is not null |
There was a problem hiding this comment.
Same here, I think we should pass a shared_ptr to std::atomic<bool> to make sure the lambda doesn't reference a non-existing stack frame.
unittests/test_utils.hpp
Outdated
| std::shared_ptr<std::promise<transaction_trace_ptr>> trx_promise = std::make_shared<std::promise<transaction_trace_ptr>>(); | ||
| std::future<transaction_trace_ptr> trx_future = trx_promise->get_future(); |
There was a problem hiding this comment.
Small detail, I think auto would be nice here (type is spelled out in the make_shared).
unittests/test_utils.hpp
Outdated
| } | ||
| feature_promise.set_value(); | ||
| }); | ||
| std::shared_ptr<std::atomic<bool>> feature_set = std::make_shared<std::atomic<bool>>(false); |
tests/test_read_only_trx.cpp
Outdated
| @@ -87,7 +87,7 @@ BOOST_AUTO_TEST_CASE(not_check_configs_if_no_read_only_threads) { | |||
| test_configs_common(specific_args, app_init_status::succeeded); | |||
| } | |||
|
|
|||
| void test_trxs_common(std::vector<const char*>& specific_args, bool test_disable_tierup = false) { | |||
| void test_trxs_common(std::vector<const char*>& specific_args, bool test_disable_tierup = false) { try { | |||
There was a problem hiding this comment.
Since we already re-indent most of the function, maybe move the try to its own line.
When interacting with
controllermany calls includingpush_transaction&preactivate_featurerequire a pending block. Updatetest_utils.hppused bytest_read_only_trx.cppto only interact withcontrollerwhen it is building a block (has a pending block).Included in this PR are try-catches around
applicationexec()since it can throw and if the exceptions are not caught then the testterminates making it very difficult to find the actual issue with the test.Resolves #1435