REM-1296: Get list of the batches #12

Art17 · 2019-04-16T12:57:38Z

Jira references

Ticket (task) — https://remmeio.atlassian.net/browse/REM-1296

Description

Get a list of batches — remme batch get-list:

Arguments	Type	Required	Description
ids	String	No	Identifiers to get a list of batches by.
start	String	No	Batch identifier to get a list of batches starting from.
limit	Integer	No	Maximum amount of batches to return.
head	String	No	Block identifier to get a list of batches from.
reverse	Bool	No	Parameter to reverse result.
node-url	String	No	Node URL to apply a command to.

$ remme batch get-list \
      --ids='6bd3382e3deef34d0bc63a7b450c88c7ae00152f5168c7b4dc4357feff6d52175209919cd0710441fa2768f4c12adf97143440ef8414bb5144b9459d78ff3e0e, 7a5daba99d5757adc997ea6a0b1b83263b3c16604dbd83c0153dc01c9fd780af4b570338c2ec60e086b1db58a4397a4dc661d6c93b0a7250fe75642e15b26e81' \
      --start=6bd3382e3deef34d0bc63a7b450c88c7ae00152f5168c7b4dc4357feff6d52175209919cd0710441fa2768f4c12adf97143440ef8414bb5144b9459d78ff3e0e \
      --limit=2 \
      --head=57a7944497ca41f424932ae6b70897e7086652ab98450d4aba6a02a2d891501460947812a41028b8041f087066df6dc7e1100c4b0e5cc94bb58b002f6950eb02 \
      --node-url=node-6-testnet.remme.io \
      --reverse
 "result": {
        "data": [
            {
                "header": {
                    "signer_public_key": "03738df3f4ac3621ba8e89413d3ff4ad036c3a0a4dbb164b695885aab6aab614ad",
                    "transaction_ids": [
                        "ed7baa5ab9bc5ef49077ddd22de3ebef56157f188980edb6401a57729f7226195d849f227fd941cc01c5eeeedd1d9cadf2e48140098422f2cb641044d971a374",
                    ]
                },
                "header_signature": "6bd3382e3deef34d0bc63a7b450c88c7ae00152f5168c7b4dc4357feff6d52175209919cd0710441fa2768f4c12adf97143440ef8414bb5144b9459d78ff3e0e",
                "trace": false,
                "transactions": [
                    {
                        "header": {
                            "batcher_public_key": "03738df3f4ac3621ba8e89413d3ff4ad036c3a0a4dbb164b695885aab6aab614ad",
                            "dependencies": [],
                            "family_name": "sawtooth_settings",
                            "family_version": "1.0",
                            "inputs": [
                                "000000a87cb5eafdcca6a8cde0fb0dec1400c5ab274474a6aa82c1c0cbf0fbcaf64c0b",
                                "000000a87cb5eafdcca6a8cde0fb0dec1400c5ab274474a6aa82c12840f169a04216b7",
                                "000000a87cb5eafdcca6a8cde0fb0dec1400c5ab274474a6aa82c1918142591ba4e8a7",
                                "000000a87cb5eafdcca6a8f82af32160bc5311783bdad381ea57b4e3b0c44298fc1c14"
                            ],
                            "nonce": "",
                            "outputs": [
                                "000000a87cb5eafdcca6a8cde0fb0dec1400c5ab274474a6aa82c1c0cbf0fbcaf64c0b",
                                "000000a87cb5eafdcca6a8f82af32160bc5311783bdad381ea57b4e3b0c44298fc1c14"
                            ],
                            "payload_sha512": "a39230835da9f4ae154abd7197f196acb9ed9a7d4d0bae8de71714bfe4636c8c6516d774e7b721b9d08d2abbafa42b01acdfe0e3c9b6d35ecef7452442e63b49",
                            "signer_public_key": "03738df3f4ac3621ba8e89413d3ff4ad036c3a0a4dbb164b695885aab6aab614ad"
                        },
                        "header_signature": "ed7baa5ab9bc5ef49077ddd22de3ebef56157f188980edb6401a57729f7226195d849f227fd941cc01c5eeeedd1d9cadf2e48140098422f2cb641044d971a374",
                        "payload": "CAESRwoic2F3dG9vdGgudmFsaWRhdG9yLmJhdGNoX2luamVjdG9ycxINcmVtbWVfYmF0Y2hlcxoSMHg5NzBkZGE2MjRlZTFhZmFh"
                    },
                ]
            },
        ],
        "head": "57a7944497ca41f424932ae6b70897e7086652ab98450d4aba6a02a2d891501460947812a41028b8041f087066df6dc7e1100c4b0e5cc94bb58b002f6950eb02",
        "paging": {
            "limit": 2,
            "next": "",
            "start": "6bd3382e3deef34d0bc63a7b450c88c7ae00152f5168c7b4dc4357feff6d52175209919cd0710441fa2768f4c12adf97143440ef8414bb5144b9459d78ff3e0e"
        }
    }
}

codecov-io · 2019-04-16T13:01:41Z

Codecov Report

Merging #12 into develop will increase coverage by 0.11%.
The diff coverage is 98.11%.

@@             Coverage Diff             @@
##           develop      #12      +/-   ##
===========================================
+ Coverage    97.31%   97.43%   +0.11%     
===========================================
  Files           55       55              
  Lines          894      973      +79     
===========================================
+ Hits           870      948      +78     
- Misses          24       25       +1

Impacted Files	Coverage Δ
cli/batch/service.py	`93.75% <100%> (+2.08%)`	⬆️
cli/batch/help.py	`100% <100%> (ø)`	⬆️
cli/batch/forms.py	`100% <100%> (ø)`	⬆️
cli/batch/cli.py	`100% <100%> (ø)`	⬆️
cli/generic/forms/fields.py	`100% <100%> (ø)`	⬆️
cli/batch/interfaces.py	`57.14% <50%> (-2.86%)`	⬇️
cli/node/cli.py	`100% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e4c83b...e8ec5b4. Read the comment docs.

dmytrostriletskyi · 2019-04-16T13:36:17Z

README.md

@@ -76,6 +76,43 @@ $ remme account get-balance \
 368440.0
 ```

+### Blockchain info


When you operate some libraries where you see blockchain info, it is ok.
When you provide the independent layer of user experience, you should care about users in own way.

I mean, In the library it is blockchain information, but CLI user expects section batch.

dmytrostriletskyi · 2019-04-16T13:36:51Z

cli/blockchain_info/cli.py

+    Get batch by id.
+    """
+    if re.match(pattern=BATCH_ID_REGEXP, string=id) is None:
+        click.echo('The following id `{batch_id}` is not valid.'.format(batch_id=id))


The following id > The following batch id.

dmytrostriletskyi · 2019-04-16T13:37:38Z

cli/blockchain_info/cli.py

+@batch_commands.command('get-single')
+def get_single(id, node_url):
+    """
+    Get batch by id.


id > identifier in all user/business-oriented texts: documentation of products, projects, code documentation, functions names.

dmytrostriletskyi · 2019-04-16T13:38:12Z

cli/blockchain_info/help.py

+
+GET_BATCH_HEAD_ARGUMENT_HELP_MESSAGE = 'Get batch by head.'
+
+GET_BATCH_REVERSE_ARGUMENT_HELP_MESSAGE = 'Reverse result.'


Remove blank lines between the same entities.

dmytrostriletskyi · 2019-04-16T13:39:11Z

cli/blockchain_info/interfaces.py

+"""
+
+
+class BlockchainInfoInterface:


The same I said in the first message. BlockchainInfo > Batch. Please, fix this anywhere: folder names, files names, classes, variables, etc. except obviously library usage.

dmytrostriletskyi · 2019-04-16T13:40:06Z

cli/blockchain_info/interfaces.py

+
+    async def get_batches(self, query=None):
+        """
+        Get all batches from REMChain.


REMChain > blockchain.

dmytrostriletskyi · 2019-04-16T13:40:40Z

cli/blockchain_info/interfaces.py

+        """
+        Get all batches from REMChain.
+
+        Args:


Args > Arguments.

dmytrostriletskyi · 2019-04-16T13:40:59Z

cli/blockchain_info/interfaces.py

+
+        Args:
+            query (dict, optional): dictionary with specific parameters
+        Returns:


Blank line between documentation sections: arguments, returns, exceptions, etc.

dmytrostriletskyi · 2019-04-16T13:41:20Z

cli/blockchain_info/service.py

+        """
+        Get batch by id.
+        """
+        # TODO: change to get_batch_by_id after corresponding changes in remme-python-library


Move TODO section of messy comments to the issues, please.

dmytrostriletskyi · 2019-04-16T13:41:37Z

cli/constants.py

@@ -3,6 +3,8 @@
 """
 ADDRESS_REGEXP = '[0-9a-f]{70}'

+BATCH_ID_REGEXP = '[0-9a-f]{128}'


Remove blank line between the same entities.

dmytrostriletskyi · 2019-04-17T10:35:16Z

cli/batch/cli.py

+@batch_commands.command('get-list')
+def get_list(id, start, limit, head, reverse, node_url):
+    """
+    List batches.


List batches > Get a list of batches.

dmytrostriletskyi · 2019-04-17T10:36:02Z

cli/batch/cli.py

+        if batch_id is not None and re.match(pattern=BATCH_ID_REGEXP, string=batch_id) is None:
+            click.echo('The following batch id `{batch_id}` is not valid.'.format(batch_id=id))
+            sys.exit(FAILED_EXIT_FROM_COMMAND)
+    if limit is not None and limit < 0:


Blank line between 50 and 51 code lines.

https://github.com/dmytrostriletskyi/nimble-python-code-style-guide#indentions

dmytrostriletskyi · 2019-04-17T10:36:19Z

cli/batch/cli.py

+    if limit is not None and limit < 0:
+        click.echo("Limit can't be negative.")
+        sys.exit(FAILED_EXIT_FROM_COMMAND)
+    if reverse is not None and reverse not in ("true", "false"):


Blank line between 54 and 53.

Use also single quotes if possible here.

dmytrostriletskyi · 2019-04-17T10:38:25Z

cli/batch/help.py

+GET_BATCH_START_ARGUMENT_HELP_MESSAGE = 'Get batch by start.'
+GET_BATCH_LIMIT_ARGUMENT_HELP_MESSAGE = 'Limit amount of batches.'
+GET_BATCH_HEAD_ARGUMENT_HELP_MESSAGE = 'Get batch by head.'
+GET_BATCH_REVERSE_ARGUMENT_HELP_MESSAGE = 'Reverse result.'


The same as here, you should describe the argument, not command. https://github.com/Remmeauth/remme-core-cli/pull/7/files#r276177736

dmytrostriletskyi · 2019-04-17T10:38:45Z

cli/batch/interfaces.py

+
+    async def get_list(self, query=None):
+        """
+        Get all batches from REMChain.


all > a list of?

anastasiia-bilova · 2019-04-17T15:52:00Z

README.md

@@ -104,6 +104,29 @@ $ remme account get-balance \
 368440.0
 ```

+### Batches


Add anchor Batches to the tree.

anastasiia-bilova · 2019-04-17T15:53:07Z

README.md

@@ -104,6 +104,29 @@ $ remme account get-balance \
 368440.0
 ```

+### Batches
+
+Get batches — ``remme batch get-list``:


Suggested change

Get batches — ``remme batch get-list``:

Get list of batches — ``remme batch get-list``:

anastasiia-bilova · 2019-04-17T15:57:43Z

README.md

+
+| Arguments | Type   |  Required | Description                                         |
+| :-------: | :----: | :-------: | --------------------------------------------------- |
+| id        | String |  No       | Get batch by its identifier.                                |


anastasiia-bilova · 2019-04-17T15:58:32Z

cli/batch/cli.py

+"""
+Provide implementation of the command line interface's batch commands.
+"""
+import sys


Make alphabetical order of packages.

Added to technical debt.

anastasiia-bilova · 2019-04-17T15:59:47Z

cli/batch/cli.py

+@click.option('--reverse', type=str, required=False, help=GET_BATCH_REVERSE_ARGUMENT_HELP_MESSAGE)
+@click.option('--node-url', type=str, help=NODE_URL_ARGUMENT_HELP_MESSAGE)
+@batch_commands.command('get-list')
+def get_list(id, start, limit, head, reverse, node_url):


Suggested change

def get_list(id, start, limit, head, reverse, node_url):

def get_batches(id, start, limit, head, reverse, node_url):

anastasiia-bilova · 2019-04-17T16:02:13Z

cli/batch/cli.py

+@click.option('--limit', type=int, required=False, help=GET_BATCH_LIMIT_ARGUMENT_HELP_MESSAGE)
+@click.option('--head', type=str, required=False, help=GET_BATCH_HEAD_ARGUMENT_HELP_MESSAGE)
+@click.option('--reverse', type=str, required=False, help=GET_BATCH_REVERSE_ARGUMENT_HELP_MESSAGE)
+@click.option('--node-url', type=str, help=NODE_URL_ARGUMENT_HELP_MESSAGE)


Suggested change

@click.option('--node-url', type=str, help=NODE_URL_ARGUMENT_HELP_MESSAGE)

@click.option('--node-url', type=str, required=False, help=NODE_URL_ARGUMENT_HELP_MESSAGE)

anastasiia-bilova · 2019-04-17T16:15:35Z

cli/batch/cli.py

+    """
+    List batches.
+    """
+    for batch_id in (id, start, head):


head should be a block identifier, not a batch identifier.

anastasiia-bilova · 2019-04-17T16:23:33Z

cli/batch/cli.py

+    batch_service = Batch(service=remme)
+    batches = loop.run_until_complete(batch_service.get_list(query))
+
+    click.echo(batches)


If you do pull from develop, you get a new function dict_to_pretty_json.

Suggested change

click.echo(batches)

click.echo(dict_to_pretty_json(batches))

ArtemFrantsiian · 2019-04-22T22:04:11Z

cli/constants.py

@@ -2,6 +2,8 @@
 Provide constants for command line interface.
 """
 ADDRESS_REGEXP = '[0-9a-f]{70}'
+BATCH_ID_REGEXP = '^[0-9a-f]{128}$'
+BLOCK_ID_REGEXP = '^[0-9a-f]{128}$'


we have in remme-client-python this: https://github.com/Remmeauth/remme-client-python/blob/17901e34f3e46c4decd6452e675f189a4f068db1/remme/models/general/patterns.py#L13. Why we need to copy/paste to another project?

dmytrostriletskyi · 2019-04-24T09:41:24Z

README.md

@@ -16,7 +16,7 @@
    * [Nodes](#nodes)
    * [Configuration file](#configuration-file)
    * [Service](#service)
-    * [Account](#account)
+    * [Batch](#batch)


Why account has been removed?

dmytrostriletskyi · 2019-04-24T09:47:31Z

README.md

+| :-------: | :----: | :-------: | --------------------------------------------------- |
+| ids       | String |  No       | Identifiers to get a list of batches by.            |
+| start     | String |  No       | Parameter to list batches starting from.            |
+| limit     | Integer|  No       | Parameter to limit amount of batches.               |


Is start an identifier of the block to start paging (inclusive) from?

Parameter to limit amount of batches > Number of transactions to return.

start - Batch identifier to start listing from.

Maximum amount of batches to return.

dmytrostriletskyi · 2019-04-24T09:48:40Z

README.md

+```bash
+$ remme batch get-list \
+      --ids=[c2eeb94926d3432e41cb5ceed078f78466389e4fe685ec958021a1368f634c035072e434e5d0ff64820dd01fde6e5afc67eb5a9ae6c48d3983ff43abe98aef6b] \
+      --node-url=159.89.104.9


Provide the most full example. As here.

dmytrostriletskyi · 2019-04-24T09:52:44Z

cli/batch/forms.py

+            raise ValidationError("Invalid reverse field. Should be either 'true' or 'false'.")
+
+    @validates('node_url')
+    def validate_node_url(self, node_url):


Use this instead.

dmytrostriletskyi · 2019-04-24T09:53:39Z

cli/batch/forms.py

+            raise ValidationError(f'The following block id `{head}` is invalid.')
+
+    @validates('reverse')
+    def validate_reverse(self, reverse):


Please, create custom fields instead of a validation function for now.

We already did the migrations to them.

dmytrostriletskyi · 2019-04-24T09:54:16Z

cli/batch/interfaces.py

+            query (dict, optional): dictionary with specific parameters
+
+        Returns:
+            List of batches.


Apply to each method you have — #18 (comment)

dmytrostriletskyi · 2019-04-24T09:55:24Z

cli/batch/service.py

+        Returns:
+            List of batches.
+        """
+        return await self.service.blockchain_info.get_batches(query=query)


Actualize the service as here — https://github.com/Remmeauth/remme-core-cli/blob/develop/cli/account/service.py#L28

dmytrostriletskyi · 2019-04-24T09:55:43Z

cli/batch/cli.py

+        'reverse': reverse,
+    }
+
+    batch_service = Batch(service=remme)


Actualize the CLI as here — https://github.com/Remmeauth/remme-core-cli/blob/develop/cli/account/cli.py#L54

dmytrostriletskyi · 2019-04-25T16:01:02Z

README.md

+      --limit=1 \
+      --head=56100bf24eed12d2f72fe3c3ccf75fe2f53d87c224d9dda6fb98a1411070b06a40fcf97fccc61cb9c88442953af6ae50344ad7773f1becc6bae108443c18c551 \
+      --reverse=true \
+      --node-url=159.89.104.9


Use common URL as in other examples — genesis.

dmytrostriletskyi · 2019-04-25T16:02:36Z

cli/batch/forms.py

+        strict=True,
+        required=False,
+        validate=[
+            validate.Range(min=1, error="Limit must be greater than 0."),


Use single quotes.

dmytrostriletskyi · 2019-04-25T16:04:09Z

cli/batch/forms.py

+    List batches validation form.
+    """
+
+    ids = IdListField(allow_none=True, required=False)


IdListField > BatchIdentifierListField.

Apply to other cases. For now, we do specific naming like, in the future when we will know then few arguments from different forms could use the same field, we will create a generic one with a generic name.

dmytrostriletskyi · 2019-04-25T16:04:36Z

cli/batch/interfaces.py

+        of batches, reversed result.
+
+        Arguments:
+            query (dict, optional): dictionary with specific parameters


dictionary with specific parameters > dictionary with specific parameters..

dmytrostriletskyi · 2019-04-25T16:05:07Z

cli/batch/service.py

+@implements(BatchInterface)
+class Batch:
+    """
+    Implements batch interface.


Implements batch interface. > Implements batch..

dmytrostriletskyi · 2019-04-25T16:05:36Z

cli/batch/interfaces.py

+        """
+        Get a list of batches.
+
+        List of batches can be filtered by identifiers, start pagging, identifier of block's head, limit


Remove List of batches can.. from the interface. Leave in the implementation.

anastasiia-bilova · 2019-04-26T13:18:54Z

README.md

+      --start=6bd3382e3deef34d0bc63a7b450c88c7ae00152f5168c7b4dc4357feff6d52175209919cd0710441fa2768f4c12adf97143440ef8414bb5144b9459d78ff3e0e \
+      --limit=2 \
+      --head=57a7944497ca41f424932ae6b70897e7086652ab98450d4aba6a02a2d891501460947812a41028b8041f087066df6dc7e1100c4b0e5cc94bb58b002f6950eb02 \
+      --node-url=node-genesis-testnet.remme.io \


Suggested change

--node-url=node-genesis-testnet.remme.io \

--reverse \

--node-url=node-genesis-testnet.remme.io

anastasiia-bilova · 2019-04-26T13:21:26Z

cli/batch/cli.py

+        'reverse': reverse,
+    }
+
+    result, errors = Batch(service=remme).get_list(


Suggested change

result, errors = Batch(service=remme).get_list(

result, errors = Batch(service=remme).get_list(query=query)

anastasiia-bilova · 2019-04-26T13:23:56Z

cli/batch/interfaces.py

+    Implements batch.
+    """
+
+    def get_list(self, query=None):


Suggested change

def get_list(self, query=None):

def get_list(self, query):

anastasiia-bilova · 2019-04-26T13:24:46Z

cli/batch/service.py

+        """
+        self.service = service
+
+    def get_list(self, query=None):


Suggested change

def get_list(self, query=None):

def get_list(self, query):

anastasiia-bilova · 2019-04-26T13:27:50Z

cli/batch/service.py

+            query['reverse'] = 'false'
+
+        try:
+            batch_list = loop.run_until_complete(


Suggested change

batch_list = loop.run_until_complete(

batch_list = loop.run_until_complete(self.service.blockchain_info.get_batches(query=query))

anastasiia-bilova · 2019-04-26T13:35:15Z

tests/batch/test_get_batches.py

+        NODE_IP_ADDRESS_FOR_TESTING,
+    ])
+
+    command_response = json.loads(result.output)


Modify test case by example.

anastasiia-bilova · 2019-04-26T13:39:23Z

tests/batch/test_get_batches.py

+    assert header is not None
+
+
+def test_get_batches_invalid_head():


Create test cases:

with valid, but non-existing ids

without node url (example)

with invalid node url (example)

node url with http (example)

node url with https (example)

dmytrostriletskyi · 2019-04-26T14:34:33Z

cli/generic/forms/fields.py

+        """
+        Validate data (list identifiers) that was passed to field.
+        """
+        value = [v.strip() for v in value.split(',')]


#18 (comment)

dmytrostriletskyi · 2019-04-26T14:36:50Z

cli/generic/forms/fields.py

+        return value
+
+
+class BatchIdField(IdField):


BatchIdField > BatchIdentifierField.

dmytrostriletskyi · 2019-04-26T14:37:01Z

cli/generic/forms/fields.py

+    """
+    Implements validation of the batch identifier.
+
+    Raises a ValidationError if the identifier does not correspond to HEADER_SIGNATURE_REGEXP.


#18 (comment)

anastasiia-bilova · 2019-05-08T15:25:46Z

README.md

+      --head=57a7944497ca41f424932ae6b70897e7086652ab98450d4aba6a02a2d891501460947812a41028b8041f087066df6dc7e1100c4b0e5cc94bb58b002f6950eb02 \
+      --node-url=node-6-testnet.remme.io \
+      --reverse
+ "result": {


Why is the result not in the dictionary?

{ 'result': ... }

anastasiia-bilova · 2019-05-08T15:31:44Z

cli/batch/cli.py

+        'node_address': str(node_url) + ':8080',
+    })
+
+    batches, errors = Batch(service=remme).get_list(


Suggested change

batches, errors = Batch(service=remme).get_list(

result, errors = Batch(service=remme).get_list(

anastasiia-bilova · 2019-05-08T15:32:35Z

cli/batch/forms.py

+
+class GetBatchesListForm(Schema):
+    """
+    Get a list of batch form.


Suggested change

Get a list of batch form.

Get a list of batches form.

anastasiia-bilova · 2019-05-08T15:33:15Z

cli/batch/help.py

+"""
+Provide help messages for command line interface's batch commands.
+"""
+BATCH_IDS_ARGUMENT_HELP_MESSAGE = 'Identifiers to get a list of batches by.'


Suggested change

BATCH_IDS_ARGUMENT_HELP_MESSAGE = 'Identifiers to get a list of batches by.'

BATCH_IDENTIFIERS_ARGUMENT_HELP_MESSAGE = 'Identifiers to get a list of batches by.'

anastasiia-bilova · 2019-05-08T15:39:41Z

cli/batch/interfaces.py

+            start (string, optional): batch identifier to get a list of batches starting from.
+            limit (int, optional): maximum amount of batches to return.
+            head (string, optional): block identifier to get a list of batches from.
+            reverse (string, optional): parameter to reverse result.


Suggested change

reverse (string, optional): parameter to reverse result.

reverse (bool, optional): parameter to reverse result.

anastasiia-bilova · 2019-05-08T15:41:36Z

cli/batch/service.py

+            head (string, optional): block identifier to get a list of batches from.
+            reverse (string, optional): parameter to reverse result.
+        """
+        reverse = '' if reverse else 'false'


Remove this line because it is in the python library.

anastasiia-bilova · 2019-05-08T16:02:59Z

tests/batch/test_get_list_batches.py

+from cli.utils import dict_to_pretty_json
+
+
+def test_get_list_batches_with_all_parameters(mocker):


Why are you using a mocker in this test? Please, send the command with all parameters to the real test node.

anastasiia-bilova · 2019-05-08T16:05:30Z

tests/batch/test_get_list_batches.py

+    result_header_signature = json.loads(result.output).get('result').get('data')[0].get('header_signature')
+
+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code
+    assert isinstance(json.loads(result.output), dict)


This check is meaningless, remove it.

anastasiia-bilova · 2019-05-08T16:11:29Z

tests/batch/test_get_list_batches.py

+    assert dict_to_pretty_json(expected_error_message) in result.output
+
+
+def test_get_list_batches_with_invalid_node_url():


This test is for a non-existing node URL, but not an invalid node URL. Update documentation.

Suggested change

def test_get_list_batches_with_invalid_node_url():

def test_get_batch_status_with_non_existing_node_url():

Create test for invalid node URL.

anastasiia-bilova · 2019-05-08T16:13:55Z

tests/batch/test_get_list_batches.py

+
+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code
+    assert expected_result == json.loads(result.output).get('result')
+


Create tests for valid, but non-existing parameters.

dmytrostriletskyi · 2019-05-10T12:48:36Z

tests/batch/test_get_list_batches.py

+
+def test_get_list_batches_with_non_existing_ids():
+    """
+    Case: get a list of batches by non existing identifiers.


non existing > non-existing.

dmytrostriletskyi · 2019-05-10T12:48:54Z

tests/batch/test_get_list_batches.py

+def test_get_list_batches_with_non_existing_ids():
+    """
+    Case: get a list of batches by non existing identifiers.
+    Expect: list of batches not found error message is returned.


list of batches not found error message is returned. > list of batches not found error message.

dmytrostriletskyi · 2019-05-10T12:49:22Z

tests/batch/test_get_list_batches.py

+    ])
+
+    expected_error_message = {
+        'errors': f'List of batches not found.',


Remove f-string if you haven't passed any argument to.

dmytrostriletskyi · 2019-05-10T12:50:59Z

tests/batch/test_get_list_batches.py

+def test_get_list_batches_with_start():
+    """
+    Case: get a list of batches by batch identifier starting from.
+    Expect: batches are returned.


batches are returned > batches are returned beginning from the batch with an identifier which matches specified start parameter or something like that.

dmytrostriletskyi · 2019-05-10T12:51:13Z

tests/batch/test_get_list_batches.py

+def test_get_list_batches_with_non_existing_start():
+    """
+    Case: get a list of batches by non-existing batch identifier starting from.
+    Expect: list of batches not found error message is returned.


list of batches not found error message is returned > list of batches not found error message.

dmytrostriletskyi · 2019-05-10T12:51:44Z

tests/batch/test_get_list_batches.py

+def test_get_list_batches_by_head():
+    """
+    Case: get a list of batches by block identifier.
+    Expect: batches are returned.


Check this and other tests for common points I have mentioned on above.

…into f-list-batches

anastasiia-bilova · 2019-05-10T13:16:35Z

README.md

+      --start=6bd3382e3deef34d0bc63a7b450c88c7ae00152f5168c7b4dc4357feff6d52175209919cd0710441fa2768f4c12adf97143440ef8414bb5144b9459d78ff3e0e \
+      --limit=2 \
+      --head=57a7944497ca41f424932ae6b70897e7086652ab98450d4aba6a02a2d891501460947812a41028b8041f087066df6dc7e1100c4b0e5cc94bb58b002f6950eb02 \
+      --node-url=node-6-testnet.remme.io \


Suggested change

--node-url=node-6-testnet.remme.io \

--reverse \

--node-url=node-6-testnet.remme.io

Node URL must be last.

anastasiia-bilova · 2019-05-10T13:24:04Z

tests/batch/test_get_list_batches.py

+        1,
+        '--head',
+        head,
+        '--node-url',


Node URL must be last.

anastasiia-bilova · 2019-05-10T13:35:22Z

tests/batch/test_get_list_batches.py

+        '--reverse',
+    ])
+
+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code


Add checks, please! (example or like this assert expected_header_signature == result_header_signature)

anastasiia-bilova · 2019-05-10T13:38:18Z

tests/batch/test_get_list_batches.py

+        NODE_IP_ADDRESS_FOR_TESTING,
+    ])
+
+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code


Add checks, please.

anastasiia-bilova · 2019-05-10T13:47:20Z

tests/batch/test_get_list_batches.py

+    assert expected_header_signature == result_header_signature
+
+
+def test_get_list_batches_with_non_existing_ids():


Make one test case using @pytest.mark.parametrize for all tests with non-existing arguments (ids, start, head), they have the same error message 'List of batches not found.'.

anastasiia-bilova · 2019-05-10T13:49:38Z

tests/batch/test_get_list_batches.py

+    assert dict_to_pretty_json(expected_error_message) in result.output
+
+
+@pytest.mark.parametrize('command_flag', ('--start', '--head'))


Suggested change

@pytest.mark.parametrize('command_flag', ('--start', '--head'))

@pytest.mark.parametrize('command_flag', ('--ids', '--start', '--head'))

Add argument ids to this test case and delete test test_get_list_batches_with_invalid_ids .

anastasiia-bilova · 2019-05-10T13:54:06Z

tests/batch/test_get_list_batches.py

+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code
+
+
+def test_get_list_batches_with_invalid_limit():


You must create a test for negative limit (example) and for an invalid limit count (example).

…into f-list-batches

anastasiia-bilova · 2019-05-14T05:42:56Z

README.md

+        }
+    ]
+}
+


Remove the blank line.

anastasiia-bilova · 2019-05-14T05:48:11Z

tests/batch/test_get_list_batches.py

+        NODE_IP_ADDRESS_FOR_TESTING,
+    ])
+
+    assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code


Add a check like this assert result_header_signature == start.

This test is for head. Result doesn't include block identifier so there is nothing to compare with head.

anastasiia-bilova · 2019-05-14T05:50:59Z

tests/batch/test_get_list_batches.py

+        'result': batch_data.get('data'),
+    }
+
+    mock_get_batch_by_ids = mocker.patch('cli.batch.service.loop.run_until_complete')


Suggested change

mock_get_batch_by_ids = mocker.patch('cli.batch.service.loop.run_until_complete')

mock_get_batches = mocker.patch('cli.batch.service.loop.run_until_complete')

Art17 requested review from dmytrostriletskyi and anastasiia-bilova April 16, 2019 12:58

dmytrostriletskyi suggested changes Apr 16, 2019

View reviewed changes

Art17 force-pushed the f-list-batches branch 3 times, most recently from e40fb89 to 44c3680 Compare April 16, 2019 15:13

dmytrostriletskyi suggested changes Apr 17, 2019

View reviewed changes

anastasiia-bilova suggested changes Apr 17, 2019

View reviewed changes

Art17 force-pushed the f-list-batches branch 6 times, most recently from d721d21 to 72976b3 Compare April 18, 2019 16:39

ArtemFrantsiian suggested changes Apr 22, 2019

View reviewed changes

dmytrostriletskyi mentioned this pull request Apr 23, 2019

Technical debt of the project #22

Open

34 tasks

Art17 force-pushed the f-list-batches branch 3 times, most recently from 6efe290 to 326892d Compare April 24, 2019 09:27

dmytrostriletskyi suggested changes Apr 24, 2019

View reviewed changes

Art17 force-pushed the f-list-batches branch 3 times, most recently from 46e20da to efad12c Compare April 25, 2019 15:38

dmytrostriletskyi suggested changes Apr 25, 2019

View reviewed changes

Art17 force-pushed the f-list-batches branch from efad12c to ba3f72e Compare April 26, 2019 12:45

anastasiia-bilova suggested changes Apr 26, 2019

View reviewed changes

dmytrostriletskyi suggested changes Apr 26, 2019

View reviewed changes

REM-1296: Get list of the batches

751cf24

Art17 force-pushed the f-list-batches branch from e805d04 to 751cf24 Compare May 8, 2019 13:45

anastasiia-bilova suggested changes May 8, 2019

View reviewed changes

Art17 force-pushed the f-list-batches branch from 650a2fb to 82e9951 Compare May 10, 2019 11:23

Merge branch 'develop' into f-list-batches

8ba1c05

Art17 force-pushed the f-list-batches branch from 82e9951 to daa5b65 Compare May 10, 2019 12:45

dmytrostriletskyi approved these changes May 10, 2019

View reviewed changes

Merge branch 'develop' of https://github.com/Remmeauth/remme-core-cli …

e8ec5b4

…into f-list-batches

Art17 force-pushed the f-list-batches branch from daa5b65 to e8ec5b4 Compare May 10, 2019 13:25

anastasiia-bilova suggested changes May 10, 2019

View reviewed changes

Artem Saprykin added 2 commits May 13, 2019 17:22

review summary

81c378b

Merge branch 'develop' of https://github.com/Remmeauth/remme-core-cli …

66aea9b

…into f-list-batches

anastasiia-bilova approved these changes May 14, 2019

View reviewed changes

Art17 merged commit 2f79f46 into develop May 14, 2019

delete-merged-branch bot deleted the f-list-batches branch May 14, 2019 13:03

anastasiia-bilova mentioned this pull request May 17, 2019

Solve weekly technical debt #56

Merged

16 tasks


		GET_BATCH_HEAD_ARGUMENT_HELP_MESSAGE = 'Get batch by head.'

		GET_BATCH_REVERSE_ARGUMENT_HELP_MESSAGE = 'Reverse result.'

	Get batches — ``remme batch get-list``:
	Get list of batches — ``remme batch get-list``:

	\| id \| String \| No \| Get batch by its identifier. \|
	\| id \| String \| No \| Identifier to get a list of batches by. \|

	def get_list(id, start, limit, head, reverse, node_url):
	def get_batches(id, start, limit, head, reverse, node_url):

	@click.option('--node-url', type=str, help=NODE_URL_ARGUMENT_HELP_MESSAGE)
	@click.option('--node-url', type=str, required=False, help=NODE_URL_ARGUMENT_HELP_MESSAGE)

	--node-url=node-genesis-testnet.remme.io \
	--reverse \
	--node-url=node-genesis-testnet.remme.io

	result, errors = Batch(service=remme).get_list(
	result, errors = Batch(service=remme).get_list(query=query)

	batch_list = loop.run_until_complete(
	batch_list = loop.run_until_complete(self.service.blockchain_info.get_batches(query=query))

		assert header is not None


		def test_get_batches_invalid_head():

	batches, errors = Batch(service=remme).get_list(
	result, errors = Batch(service=remme).get_list(

	BATCH_IDS_ARGUMENT_HELP_MESSAGE = 'Identifiers to get a list of batches by.'
	BATCH_IDENTIFIERS_ARGUMENT_HELP_MESSAGE = 'Identifiers to get a list of batches by.'

	reverse (string, optional): parameter to reverse result.
	reverse (bool, optional): parameter to reverse result.

		from cli.utils import dict_to_pretty_json


		def test_get_list_batches_with_all_parameters(mocker):

		assert dict_to_pretty_json(expected_error_message) in result.output


		def test_get_list_batches_with_invalid_node_url():

	def test_get_list_batches_with_invalid_node_url():
	def test_get_batch_status_with_non_existing_node_url():


		assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code
		assert expected_result == json.loads(result.output).get('result')

	--node-url=node-6-testnet.remme.io \
	--reverse \
	--node-url=node-6-testnet.remme.io

		assert expected_header_signature == result_header_signature


		def test_get_list_batches_with_non_existing_ids():

		assert dict_to_pretty_json(expected_error_message) in result.output


		@pytest.mark.parametrize('command_flag', ('--start', '--head'))

		assert PASSED_EXIT_FROM_COMMAND_CODE == result.exit_code


		def test_get_list_batches_with_invalid_limit():

REM-1296: Get list of the batches #12

REM-1296: Get list of the batches #12

Conversation

Art17 commented Apr 16, 2019 • edited Loading

Jira references

Description

codecov-io commented Apr 16, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anastasiia-bilova Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Art17 Apr 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anastasiia-bilova Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Art17 commented Apr 16, 2019 •

edited

Loading

codecov-io commented Apr 16, 2019 •

edited

Loading

anastasiia-bilova Apr 17, 2019 •

edited

Loading

Art17 Apr 18, 2019 •

edited

Loading

anastasiia-bilova Apr 17, 2019 •

edited

Loading

	mock_get_batch_by_ids = mocker.patch('cli.batch.service.loop.run_until_complete')
	mock_get_batches = mocker.patch('cli.batch.service.loop.run_until_complete')