-
Notifications
You must be signed in to change notification settings - Fork 297
Refactor TransferAsset acceptance test #1482
Conversation
- Add IntegrationTestFramework::sendTxAwait - Reuse AcceptanceFixture primitive in TA test - Move out primitive methods in fixture - Fix WithOnlyCanTransferPerm/WithOnlyCanReceivePerm tests Signed-off-by: Kitsu <mail@kitsu.me>
f766c30
to
a701b3a
Compare
* @when execute tx with TransferAsset command | ||
* @then there is an empty proposal | ||
*/ | ||
TEST_F(TransferAsset, WithOnlyCanTransferPerm) { | ||
TEST_F(TransferAsset, DISABLED_WithOnlyCanTransferPerm) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please specify in a comment the reason why the test is disabled. If the test can be enabled only after resolution of some other issue, denote it as a todo comment with Jira task id.
* @when execute tx with TransferAsset command | ||
* @then there is an empty proposal | ||
*/ | ||
TEST_F(TransferAsset, WithOnlyCanReceivePerm) { | ||
TEST_F(TransferAsset, DISABLED_WithOnlyCanReceivePerm) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please specify in a comment the reason why the test is disabled. If the test can be enabled only after resolution of some other issue, denote it as a todo comment with Jira task id.
.sendTxAwait(create_asset, check(1)) | ||
.sendTxAwait(add_assets, check(1)) | ||
.sendTxAwait(make_transfer, check(1)) | ||
.sendQuery(make_query(kUserId), check_balance(kUserId, kLeft)) | ||
.sendQuery(make_query(kUser2Id), check_balance(kUser2Id, kForTransfer)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite the test does not fail, both queries (lines 378, 379) cause postgres' errors:
[2018-06-21 22:45:45.536809398][th:20689][info] IntegrationTestFramework send query
[2018-06-21 22:45:45.543091771][th:20689][error] PostgresWsvQuery ERROR: invalid input value for enum grantable_perm: ""
LINE 1: ...dmin@test' AND account_id = 'user@test' AND permission = '';
^
[2018-06-21 22:45:45.544989785][th:20689][info] IntegrationTestFramework send query
[2018-06-21 22:45:45.552626695][th:20689][error] PostgresWsvQuery ERROR: invalid input value for enum grantable_perm: ""
LINE 1: ...n@test' AND account_id = 'usertwo@test' AND permission = '';
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be resolved with IR-1464
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall but fix test plz.
.finish()) | ||
.checkBlock( | ||
[](auto &block) { ASSERT_EQ(block->transactions().size(), 0); }) | ||
.sendTxAwait(makeFirstUser(), check(1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this test doesn't work for both me and CI, fix it please.
@@ -94,7 +94,7 @@ auto AcceptanceFixture::baseTx() | |||
|
|||
auto AcceptanceFixture::baseQry() | |||
-> decltype(base(TestUnsignedQueryBuilder())) { | |||
return base(TestUnsignedQueryBuilder()); | |||
return base(TestUnsignedQueryBuilder()).queryCounter(tx_counter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plz rename tx_counter
-> query_counter
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's the same counter for both tx and query, I may rename it to just counter
Signed-off-by: Kitsu <mail@kitsu.me>
121698e
to
d02f4f5
Compare
Signed-off-by: Kitsu <mail@kitsu.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was able to compile only with the fix:
iff --git a/irohad/synchronizer/CMakeLists.txt b/irohad/synchronizer/CMakeLists.txt
index 1f31648..f27f01e 100644
--- a/irohad/synchronizer/CMakeLists.txt
+++ b/irohad/synchronizer/CMakeLists.txt
@@ -3,7 +3,7 @@ add_library(synchronizer
)
target_link_libraries(synchronizer
- shared_model_interfaces
+ shared_model_proto_backend
rxcpp
logger
)
Signed-off-by: Kitsu <mail@kitsu.me>
- Add IntegrationTestFramework::sendTxAwait - Reuse AcceptanceFixture primitive in TA test - Move out primitive methods in fixture - Fix (and rename accordingly) WithOnlyCanTransferPerm/WithOnlyCanReceivePerm tests - Rename tx_counter in AcceptanceFixture Signed-off-by: Kitsu <mail@kitsu.me>
Description of the Change
Also, I've tried to remove
Role::kAddAssetQty
from the first user but it seems to have some bug. Feel free to review/comment it as wellBenefits
Lesser simpler code -> lesser bugs
Possible Drawbacks
None?
Usage Examples or Tests
make transfer_asset_test && test_bin/transfer_asset_test