Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Adding support for Tuples and adding example tests for nested containers #1068

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

bradlhart
Copy link
Contributor

Change Description

Adding support in eosjs-serialize.ts to serialize tuples.
Added several tests for different types of nested containers.

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

Copy link

@plroblox plroblox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use first-second for pair, not key-value, see Farhad's EOSIO/eos#11006.

it('should test Multi Index nested containers', async () => {
jest.setTimeout(30000);

await api.getAbi('nestcontnmi');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment here to specify the contract account nestcontnmi is from launching eos/tests/nested_container_multi_index_test.py and also specify the related nested_container_multi_index.cpp contract can be found from eos/unittests/test-contracts/nested_container_multi_index ?


// Test action for set< pair< uint16_t, uint16_t >>
await api.transact({
actions: [ api.with('nestcontnmi').as('alice').setstp('alice', [{'key': 69, 'value': 129}, {'key': 69, 'value': 129}]) ]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSON input format here is NOT correct, for pair, 'first' and 'second' should be used, insstead of 'key' and 'value', Farhad is in the middle of updating the related .py script, see EOSIO/eos#11006, so here all of the lines related to pair shall be changed.

Please note that the latest eosio-cpp built from [develop] branch of eosio.cdt shall be used, however, you can just run his *.py script, because he also modified/updated related .WASM and .abi generated by the latest eosio-cpp

// Test action for optional< pair< uint16_t, uint16_t >>
await api.transact({
actions: [ api.with('nestcontnmi').as('alice').setop('alice', {'key': 60, 'value': 61}) ]
}, { blocksBehind: 3, expireSeconds: 30 });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use first, second for setop

// Test action for vector< pair< uint16_t, uint16_t >>
await api.transact({
actions: [ api.with('nestcontnmi').as('alice').setvp('alice', [{'key': 69, 'value': 129}, {'key': 69, 'value': 129}]) ]
}, { blocksBehind: 3, expireSeconds: 30 });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use first, second for setvp

// Test action for map< pair< uint16_t, uint16_t >>
await api.transact({
actions: [ api.with('nestcontnmi').as('alice').setmp('alice', [{'key': 36, 'value': {'key': 300, 'value': 301}}, {'key': 37, 'value': {'key': 600, 'value': 601}} ]) ]
}, { blocksBehind: 3, expireSeconds: 30 });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for pair-part, use firt-second, not key-value for this setmp

// Test action for pair< set< uint16_t >>
await api.transact({
actions: [ api.with('nestcontnmi').as('alice').setpst('alice', {'key': 20, 'value': [200, 200, 202]}) ]
}, { blocksBehind: 3, expireSeconds: 30 });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow the same rule to use first-second only for setpst,setpv, setpo, setpm,setpp,setpt

await api.transact({
actions: [ api.with('nestcontnmi').as('alice').settp('alice', [127, {'key':18, 'value':28}, {'key':19, 'value':29}]) ]
}, { blocksBehind: 3, expireSeconds: 30 });

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same change for settp

jest.setTimeout(30000);

await api.getAbi('nestcontn2kv');

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as previous api.getApi(), here please add similar comment to explain nestcotn2kv and where is the related nested_container_kv.cpp contract


// Test action for set< pair< uint16_t, uint16_t >>
await api.transact({
actions: [ api.with('nestcontn2kv').as('alice').setstp(1, [{'key': 69, 'value': 129}, {'key': 69, 'value': 129}]) ]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same change for pair, first-second shall be used, ...

Copy link

@plroblox plroblox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to make sure both Write and Read operations are tested, you have used get table and get kv table, so no action is to be taken

}, { blocksBehind: 3, expireSeconds: 30 });

const transaction = await rpc.get_table_rows({
code: 'nestcontnmi',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, 'get table' is important, those setxx actions will pack/Write data into nodeos block-chain, get table and get kv table will unpack/Read data from nodeos, I just want to make sure they are used explicitly, no correction action

Copy link

@plroblox plroblox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is ok now, JSON input formats for single-layer container, struct, and shortcut JSON input formats should be added later on after other developers' work is done

@bradlhart
Copy link
Contributor Author

When this is merged in, revert #1104 to ensure nested containers is tested nightly on integration tests

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants