Reduce test fixtures size to speed up tests

ixje commented 6 years ago

Current behavior

travis-ci takes > 15 minutes to run all tests. One reason is that the tests rely on a fixture database that's > 500MB in size. The fixture database is basically a zipped up testnet blockchain folder. This big size is the result of having testcases deployed on the regular TestNet, which then have many unnecessary blocks added to the fixtures in between different tests created on that network.

We believe that the fixture size can be easily reduced to < 50 MB by recreating all tests on a special private TestNet. This testnet should be based on neo-privatenet-docker The image should only be spun up when adding new tests that requires events or interactions on a full blockchain (e.g. smart contract storage tests) to keep the database to the minimal size necessary.

TODO:

[x] create an inventory of all tests to be ported to the private testnet (hint: look at what derives from: [BlockchainFixtureTestCase](https://github.com/CityOfZion/neo-python/blob/master/neo/Utils/BlockchainFixtureTestCase.
[x] migrate all tests to a new private testnet docker image (based on neo-privatenet-docker)
[x] make testnet docker image publicly available
[x] create new fixture file, get it on CoZ AWS (ask @localhuman), update all test cases.
[x] create PR to neo-python

dauTT commented 6 years ago

Hi ixje, I would be happy to help you with this issue. Shall I start with the creation of the inventory? Question: how exactly do you want to migrate all tests using docker? A concrete example would help here as I have basic knowledge of docker. Thanks.

ixje commented 6 years ago

Hi @dauTT that would be great!

To explain a bit more; almost all of the current tests are created on the official TestNet. Which means that there are smart contracts deployed on the official TestNet that have been interacted with in the past and we then confirm with our tests that we still get the same results.

We want to re-create those smart contracts (as the sources are likely lost @localhuman ?), but then deploy them on a new private network created with the neo-private-docker image. Have a look at https://github.com/CityOfZion/neo-privatenet-docker to see what that is about.

So the first step would be to try re-create the smart contracts belonging to the specific tests that are currently in neo-python. You can test and validate your new contracts either on the testnet or using a neo-private-docker.
once that is done we create a new private network where we deploy all smart contracts needed for the tests, do the required invocations (=interactions with the contracts) and then create a snapshot of the docker image to keep it small (because if you keep it running, it will keep generating blocks in the background and the blockchain will then unnecessarily increase in size).

I hope that makes sense. Let me know if I can clarify anything else.

dauTT commented 6 years ago

Thanks ixje for the clarification. I believe I understand now what to do. I will proceed with migrating only one test (e.g, neo-python/neo/SmartContract/tests/test_smart_contract2.py) and then after cross checking with you again I will migrate all the remaining test cases.

ixje commented 6 years ago

I'll be away for 4 days so I can't give you any support. If you want you can reach out to others like metachris and localhuman on Discord: https://discordapp.com/invite/b8QNXwD/

dauTT commented 6 years ago

Hi ixje,

as @localhuman mentioned on Discord, he doesn't have the source code of the smart contracts used in the neo-python BlockchainFixture test cases. So he suggested that it may be a good idea to modify the test cases and use any smart contract in https://github.com/CityOfZion/neo-boa/tree/master/boa_test/example. I did that for a test case and was able to migrated to the private neo-private-docker. I will share the result of my test with you most likely tomorrow.

jseagrave21 commented 6 years ago

@dauTT Hello, I am wondering if you would like some help with this issue. Not sure how to collaborate in GitHub (I am new to GH) but I'd love to if that is possible.

ixje commented 6 years ago

sure, just drop me a message on Discord (same name as here)

dauTT commented 6 years ago

@jseagrave21 , you are most welcome to join. We will definitely need help here. :)

@ixje : I have create an image of the neo-privnet on the docker.hub:

docker pull dautt/neo-privnet-unittest:v0.0.0

On this privnet I have deploy the smart contract (sample2.py) as described by localhum in his guideline. You can search for the contract by using this command:

contract search test

I have tested the image with neo-python cc4da934d3afc2ad46e1deb98ce638f0afd157bc (tag: v0.7.3). If we take that particular version and replace the 2 files:

with the corresponding one in my fork:

we see that the test cases in test_smart_contract.py for the block 9369 will pass by running the following commands:

1) run the privnet: docker run --rm -d --name neo-privnet-unittest -p 20333-20336:20333-20336/tcp -p 30333-30336:30333-30336/tcp dautt/neo-privnet-unittest:v0.0.0 2) activate the virtual environment 3) python prompt.py -p 4) After the local blockchain is in sync with the privnet, exit from the prompt. 5) python3 -m unittest /neo/SmartContract/tests/test_smart_contract.py

Currently the latest neo-python version and the neo-privnet image are not compatible. So my branch move-unittest-to-privnet-478 is also not working properly. Do you know the reason for this discrepancy?

I will try to fix this issue as well, but meanwhile you can already review whatever has been done so far and let us know if we are in right direction. Thanks.

(Below, in the attachment, we find more information about the test case:) POC test case.txt

jseagrave21 commented 6 years ago

@dauTT Could you specify exactly what you mean by "the latest neo-python version and the neo-privnet image are not compatible"? What errors/failures/warnings are you receiving? I just followed your instructions and everything seems to run okay for me. Your branch of the test_smart_contract passed.

jseagrave21 commented 6 years ago

After talking with @ixje, I think I understand how to proceed. I created a branch for this project but we will have to document our changes on this issue because there is no way we can think of to all edit the same code at the same time. We will need to verify every test manually on the @dauTT's privnet before we can create the final snapshot. I started to work on the next test: test_smart_contract2.py located here: https://github.com/jseagrave21/neo-python/blob/16133d5236f9e4c7218bf6a5e56480d16eaf8762/neo/SmartContract/tests/test_smart_contract2.py Unfortunately, when I ran it I get this error:

ERROR: test_b_invocation (neo.SmartContract.tests.test_smart_contract2.SmartContractTest2)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/users/jseag/Neo/neo-python-travis/neo/SmartContract/tests/test_smart_contract2.py", line 26, in test_b_invocation
    result = self._blockchain.Persist(block)
  File "/mnt/c/users/jseag/Neo/neo-python-travis/neo/Implementations/Blockchains/LevelDB/TestLevelDBBlockchain.py", line 71, in Persist
    if prevTx.outputs[input.PrevIndex].AssetId.ToBytes() == Blockchain.SystemShare().Hash.ToBytes():
AttributeError: 'NoneType' object has no attribute 'outputs'

I am not sure why I am receiving this error. I will keep working.

I am also trying to think about a way to keep track of tests we have "updated". Maybe a Google Doc? What do you guys think? @ixje @dauTT

ixje commented 6 years ago

@dauTT looks good what you have so far! At the end of the road we'll have to keep most of your deletions from BlockchainFixtureTestCase.py, but for now it's a good approach for testing.

I followed all your instructions (nice a clear 👍) but I do not face any errors

(venv) Eriks-Air:neo-python erik$ python -m unittest neo/SmartContract/tests/test_smart_contract.py
[I 180801 20:39:28 LevelDBBlockchain:115] Created Blockchain DB at /Users/erik/.neopython/Chains/privnet
...
----------------------------------------------------------------------
Ran 3 tests in 0.408s

OK

The only difference is that I took the latest master 0.7.5 and applied your edits, where you run 0.7.3.

ixje commented 6 years ago

A document could work, or you post whatever test file you're going to work on next here. It's probably best to discuss it on Discord or alike though. Looking forward to your contributions 👍

dauTT commented 6 years ago

I guess we can proceed to migrate the remaining test cases:

test_LevelDBBlockchain.py (Done, assigned to @dauTT)
test_leveldb.py (Done, assigned to @dauTT)
test_interop_blockchain.py (Done, assigned to @dauTT)
test_rest_api.py (Done, assigned to @dauTT)
test_json_invoke_rpc_api.py (Done, assigned to @jseagrave21)
test_json_rpc_api.py (In progress, assigned to @jseagrave21)
test_smart_contract.py (Closed, @ixje has done the review)
test_smart_contract3.py (Done, assigned to @jseagrave21)
test_smart_contract2.py (Done, assigned to @jseagrave21 )

There are not so many. So if you don't mind I can track them here. Please let me know if I have missed anything.

If we need to deploy more contracts on the privnet docker let me know. I can do it for you and update the docker images.

dauTT commented 6 years ago

Hi @jseagrave21 , are you interested on working on the following test modules (4,5,7) as well?

jseagrave21 commented 6 years ago

Sure thing. I am stuck on test_smart_contract2.py. Would you mind taking a look at it? I still get the error I mentioned previously. I will temporarily move on to test_smart_contract3.py

dauTT commented 6 years ago

Okay, I will look at it maybe tomorrow. Thanks! :)

jseagrave21 commented 6 years ago

@dauTT I completed the update for test_smart_contract2.py https://github.com/jseagrave21/neo-python/blob/patch-5/neo/SmartContract/tests/test_smart_contract2.py

jseagrave21 commented 6 years ago

@dauTT For test_smart_contract3, I think it is referencing a block which deploys a smart contract. I asked @ixje if there was a way to search the blocks in the privatenet for one which deploys a SC and he couldn't think of one. I am wondering if you think this test will require us to write a test SC and deploy it on the privatenet then test it with an updated version of test_smart_conract3?

dauTT commented 6 years ago

Hi @jseagrave21 ,

I have already deployed a smart contract on privnet. It is related to block 9369. For more information you can look at the attachment that I have posted above (POC test case.txt). With respect to the a new SM contract deployment, If it make your life easy, just do it on your privnet for testing. But I will first try to work with what we already have. If you decide to use a new smart contract, remember that once I will deploy on the privnet to create the new image, your test cases won't work anymore and you will need to fix them again. So Unfortunately we need to do double work but if you have figure out how to fix them once, then the second time is quicker.

jseagrave21 commented 6 years ago

Oh that is great! I will see if I can work with that.

jseagrave21 commented 6 years ago

@dauTT @ixje okay test_smart_contract3 is done I think. Please review to make sure it is still doing what we want it to do.

https://github.com/jseagrave21/neo-python/blob/patch-5/neo/SmartContract/tests/test_smart_contract3.py

dauTT commented 6 years ago

I am still working on test_rest_api.py. It take a little bit longer as expected because the unit test migration force me to get deeper into the understanding of NEP5 standard and transfering token between different accounts. I have already deployed similar contract on the privnet and I am trying to migrate additional tests.

@jseagrave21 , could you please give an update from your side? Thanks.

jseagrave21 commented 6 years ago

@dauTT I am having similar issues. I have been struggling with understanding what the tests are trying to do and what the error codes mean. If you are having to deploy additional contacts, will you share an updated privnet for me to use? Also maybe some changes in the tests that have resulted in success?

dauTT commented 6 years ago

@jseagrave21 , I will be able to share something tomorrow. :)

dauTT commented 6 years ago

Hi @jseagrave21 ,

I have created a new image:

docker pull dautt/neo-privnet-unittest:v0.0.4

So far I have deployed a couple of SM on the privnet. You can find them on the location

root/.neopython/UnitTest-SM of the running docker container.

I have modified the test module, test_rest_api.py as follow:

1) test_rest_api2_Dau.txt 2) BlockchainFixtureTestCase_Dau.txt

In order for you to run the test case you will need the following fixtures which are basically create from the above docker image:

fixtures_v8.tar.gz notif_fixtures_v8.tar.gz

Relevant transactions used in the test case have been recorded in the following attached file:

test relevant transactions.txt

I am very close to finish the migration of the test_rest_api.py. Hopefully, It will done one or two day.

Let me know if you need further help.

jseagrave21 commented 6 years ago

@dauTT Awesome! I am digging in!

jseagrave21 commented 6 years ago

@dauTT I am trying to test my version of test_json_invoke_rpc_api.py but when I do I get this error from the BlockchainFixtureTestCase_Dau

ERROR: setUpClass (neo.api.JSONRPC.test_json_invoke_rpc_api.JsonRpcInvokeApiTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/users/jseag/neo/neo-python-travis/neo/Utils/BlockchainFixtureTestCase_Dau.py", line 35, in setUpClass
    response.raise_for_status()
  File "/mnt/c/users/jseag/Neo/neo-python-travis/venv/share/python-wheels/requests-2.18.4-py2.py3-none-any.whl/requests/models.py", line 935, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://s3.us-east-2.amazonaws.com/cityofzion/fixtures/fixtures_v8.tar.gz

Am I doing something wrong or do you need to give me access to the new fixture file?

dauTT commented 6 years ago

Hi @jseagrave21 ,

if you look how the test cases are setup in test_rest_api2_Dau.py or test_rest_api2.py,

    def setUpClass(cls):

        super(NotificationDBTestCase, cls).setUpClass()

        if not os.path.exists(cls.N_FIXTURE_FILENAME):
            logzero.logger.info(
                "downloading fixture notification database from %s. this may take a while" % cls.N_FIXTURE_REMOTE_LOC)

            response = requests.get(cls.N_FIXTURE_REMOTE_LOC, stream=True)

            response.raise_for_status()
            with open(cls.N_FIXTURE_FILENAME, 'wb+') as handle:
                for block in response.iter_content(1024):
                    handle.write(block)

you can avoid to download the fixtures from AWS by putting the fixture archives here :

os.path.join(settings.DATA_DIR_PATH, 'docker-privnet/Chains/')

From my understanding, after we have migrated all the test cases to the privnet, we will create new fixture archives and ask @localhuman to upload them to AWS. Until then we have to do a workaround.

ixje commented 6 years ago

Correct @dauTT. Once we understand all tests, have the steps written down and source files saved (let's not make the same mistake twice ;)), then we reset the network, redeploy everything, execute the steps and update the tests where need one final time to match the new blocks and keep the blockchain as small (==short) as possible. This will then be our new testfixture file that we'll ask localhuman to upload. I'm confident we can get a 10x file size reduction improving the build time massively.

jseagrave21 commented 6 years ago

@dauTT Okay, I am almost done with test_json_invoke_rpc_api.py but I am running into an issue with test_invoke_5. I think it is because I am trying to use the wrong script for the invokescript. Where does the invokescript come from? Is it something I need to generate? Specifically, I am referring to here Every other test within test_json_invoke_rpc_api.py is now passing.

dauTT commented 6 years ago

Thanks @jseagrave21 , I will review your test cases soon and give you feedback.

@ixje , I need your help here to understand this strange behavior in test 9 of this module, test_rest_api.py

def test_9_by_tx(self):
        mock_req = requestMock(path=b'/tx/0x4c927a7f365cb842ea3576eae474a89183c9e43970a8509b23570a86cb4f5121')
        res = self.app.get_by_tx(mock_req, '0x4c927a7f365cb842ea3576eae474a89183c9e43970a8509b23570a86cb4f5121')
        jsn = json.loads(res)
        self.assertEqual(jsn['total'], 1)
        results = jsn['results']
        self.assertEqual(len(results), 1)

the transaction tx = 4c927a7f365cb842ea3576eae474a89183c9e43970a8509b23570a86cb4f5121 is included in block 455553. You can see more information about this test case here: 1) http://testnotifications.neeeo.org/v1/transaction/4c927a7f365cb842ea3576eae474a89183c9e43970a8509b23570a86cb4f5121

2) http://testnotifications.neeeo.org/v1/notifications/block/455553

If you look at the get_by_tx method you will see that the notification block (block_notifications) is actually stored at

height -1 = 455553-1 = 455552

def get_by_tx(self, request, tx_hash):
        request.setHeader('Content-Type', 'application/json')

        bc = Blockchain.Default()  # type: Blockchain
        notifications = []
        try:
            hash = UInt256.ParseString(tx_hash)
            tx, height = bc.GetTransaction(hash)
            print (tx, ' height =', height)
            if not tx:
                return self.format_message("Could not find transaction for hash %s" % (tx_hash))
            block_notifications = self.notif.get_by_block(height - 1)
            print ("block_notifications:", block_notifications)
            for n in block_notifications:
                if n.tx_hash == tx.Hash:
                    notifications.append(n)
        except Exception as e:
            logger.info("Could not get tx with hash %s because %s " % (tx_hash, e))
            return self.format_message("Could not get tx with hash %s because %s " % (tx_hash, e))

        return self.format_notifications(request, notifications)

block_notifications[0].ToJson() = {
'type': 'SmartContract.Runtime.Notify', 
'contract': '0x73d2f26ada9cd95861eed99e43f9aafa05630849', 
'block': 455552, 
'tx': '0x4c927a7f365cb842ea3576eae474a89183c9e43970a8509b23570a86cb4f5121', 
'notify_type': 'transfer', 
'addr_to': 'AL5e5ZcqtBTKjcQ8reiePrUBMYSD88v59a', 
'addr_from': 'AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM', '
amount': 12000000000000
}

In particular we can notice that the test transaction is stored at block 455553 but the corresponding notifications block is at 455552

If I use my fixtures and consider the following transaction which happened at block 9583 :

neo> wallet tkn_send b9fbcff6e50fd381160b822207231233dd3c56c2 AK2nJJpJr6o664CWJK
i1QRXjqeic2zRp8y AXpNr3SDfLXbPHNdqxYeHK5cYpKMHZxMZ9 10000                       
Used 2.807 Gas 

-----------------------------------------------------------
Will transfer 10000.00000000 NXT2 from AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y to AXpNr3SDfLXbPHNdqxYeHK5cYpKMHZxMZ9
Transfer fee: 0.0001 
-------------------------------------------------------------

[Password]> ***                                                                 
[I 180809 23:50:52 Transaction:615] Verifying transaction: b'a2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8' 
Relayed Tx: a2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8

and use it in test 9:

def test_9_by_tx(self):
        mock_req = requestMock(path=b'/tx/0xa2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8')
        res = self.app.get_by_tx(mock_req, '0xa2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8')
        jsn = json.loads(res)
        self.assertEqual(jsn['total'], 1)
        results = jsn['results']
        self.assertEqual(len(results), 1)

the test fail because the block_notifactions ( related to Tx: a2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8) are stored at 9583 instead of 9582 = 9583 -1.

block_notifications[0].ToJson() = {
'type': 'SmartContract.Runtime.Notify', 
'contract': '0xb9fbcff6e50fd381160b822207231233dd3c56c2', 
'block': 9583, 
'tx': '0xa2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8', 
'notify_type': 'transfer', 
'addr_to': 'AXpNr3SDfLXbPHNdqxYeHK5cYpKMHZxMZ9', 
'addr_from': 'AK2nJJpJr6o664CWJKi1QRXjqeic2zRp8y', 'amount': 1000000000000}

Do you have any explanation for this strange behavior?

(more information about the the tx = a2a37fd2ab7048d70d51eaa8af2815e0e542400329b05a34274771174180a7e8 can be found in the fixture archives or in the docker image: docker pull dautt/neo-privnet-unittest:v0.0.4)

ixje commented 6 years ago

@dauTT I've asked around for you. Apparently a fix was merged some time ago that solves the issue. However, because the notificationDB fixtures were created with the 'bug' in it we never noticed that we had to update RestApi(). So the new behaviour you identified is actually correct and we need to fix RestApi.

jseagrave21 commented 6 years ago

@ixje @dauTT Based on issue #554 , I reviewed my migrated tests and noticed that some tests are behaving similarly. Specifically in test_json_rpc_api.py (note: this test is not fully migrated) in test_get_block_int, in order for me to get the test to pass the index I query is 10 and the index I receive is 9. For comparison the original test is here My "migrated" test is here Is this behavior related to the aforementioned issue?

dauTT commented 6 years ago

Thanks @ixje for solving this puzzle. :+1:

I have cross-check again using the latest notification file:

"NotificationBootstrapFile":"https://s3.us-east-2.amazonaws.com/cityofzion/bootstrap_testnet/TestNotif1413xxx.tar.gz"

Indeed the notification block of the transaction, 0x4c927a7f365cb842ea3576eae474a89183c9e43970a8509b23570a86cb4f5121, is 455553 as we would have expected. So nothing to worry and we can move on with the migration of the unit tests.

@jseagrave21, I will try review everything by the end of this week.

dauTT commented 6 years ago

@jseagrave21 , I have reviewed the following:

test_json_invoke_rpc_api.py (Good. We still need to fix test_invoke_5)

test_json_rpc_api.py (Still some work to do)


----------------------------------------------------------------------
Ran 48 tests in 18.827s

FAILED (failures=15, errors=1)


In regard to your question related to `test_get_block_int`, I still need to investigate further.

- test_smart_contract3.py (Good. :+1: )
- test_smart_contract2.py (Good :+1: )

dauTT commented 6 years ago

@ixje ,

In order for me to migrate this test case,

def test_pagination_for_addr_results(self):
        mock_req = requestMock(path=b'/addr/AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM')
        res = self.app.get_by_addr(mock_req, 'AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM')
        jsn = json.loads(res)
        self.assertEqual(jsn['total'], 1027)
        results = jsn['results']
        self.assertEqual(len(results), 500)
        self.assertEqual(jsn['total_pages'], 3)

        mock_req = requestMock(path=b'/addr/AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM?page=1')
        res = self.app.get_by_addr(mock_req, 'AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM')
        jsn = json.loads(res)
        self.assertEqual(jsn['total'], 1027)
        results = jsn['results']
        self.assertEqual(len(results), 500)

        mock_req = requestMock(path=b'/addr/AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM?page=2')
        res = self.app.get_by_addr(mock_req, 'AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM')
        jsn = json.loads(res)
        self.assertEqual(jsn['total'], 1027)
        results = jsn['results']
        self.assertEqual(len(results), 500)

        mock_req = requestMock(path=b'/addr/AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM?page=3')
        res = self.app.get_by_addr(mock_req, 'AFmseVrdL9f9oyCzZefL9tG6UbvhPbdYzM')
        jsn = json.loads(res)
        self.assertEqual(jsn['total'], 1027)
        results = jsn['results']
        self.assertEqual(len(results), 27)

I will need to create more than 1000 transfers. I am doing that automatically taking for granted that issue #558 will be fixed. In case issue # 558 is not a bug but a feature, I won't be able to generate the test case automatically. In that case we will need to do a lot of manual work if we want to preserve the test case. :(

jseagrave21 commented 6 years ago

@dauTT Thank you for the review. When I asked @localhuman about the invokeScript he sent me here but I am not sure how to implement this to build the script in order to retrieve the information to satisfy test_invoke_5 as written. Could you take a look and maybe we can discuss on Discord (in real time). It should be simple but I don't understand enough to make it work.

dauTT commented 6 years ago

@jseagrave21 , I will have a look at test_invoke_5 in the upcoming days. If needed we can arrange a video call.

From my side, I manage to fully migrate test_rest_api.py to the privnet.

Please use the new update fixtures archives and image:

fixtures_v8.tar.gz notif_fixtures_v8.tar.gz

docker pull dautt/neo-privnet-unittest:v0.0.18

Relevant transactions added in the new image are to be found in the following attachment:

test relevant transactions 3.txt

@ixje, we are getting close to the completion of this task.

To summarize the next steps: 1) Dau will look at test_invoke_5 (Done) 2) @jseagrave21 will complete the migration of test_json_rpc_api.py 3) Dau will integrate @jseagrave21 work and make sure that all test cases are running correctly 4) @ixje will do the final review 5) ask @localhuman to upload the new fixture archives to AWS. 6) documentation 7) close the task

jseagrave21 commented 6 years ago

@dauTT I like your plan. My only question at this point is the issue I saw with test_get_block_int. When I query block 10, block 9 is returned as indicated in my previous comment. Have you investigated this? Do I need to create a separate issue from the REST API -1 issue?

jseagrave21 commented 6 years ago

I did some more testing and I think the right script for test_invoke_5 is 0d444601ee23a8882f6fce312d5a0b59ea02f323f7f391e6ef3341669f411cbe. However, this does not return the expected values (i.e. bytearray(b'NEX Template V2'), bytearray(b'NXT2'), 8). This is its raw return before it is sent to ContractParameter.FromJson:

'stack': [{'type': 'ByteArray', 'value': '444601ee23a8882f6fce312d5a'}, {'type': 'ByteArray', 'value': '59ea02f323f7f391e6ef33'}, {'type': 'ByteArray', 'value': '669f411cbe'}]

Hopefully this helps.

dauTT commented 6 years ago

Thanks @jseagrave21 for the hint! :+1:

test_invoke_5 fixed:

def test_invoke_5(self):
        test_script = "00046e616d6567c2563cdd3312230722820b1681d30fe5f6cffbb9000673796d626f6c67c2563cdd3312230722820b1681d30fe5f6cffbb90008646563696d616c7367c2563cdd3312230722820b1681d30fe5f6cffbb9" 

        req = self._gen_rpc_req("invokescript", params=[test_script])
        mock_req = mock_request(json.dumps(req).encode("utf-8"))
        res = json.loads(self.app.home(mock_req))
        self.assertEqual(res['result']['state'], VMStateStr(VMState.HALT + VMState.BREAK))

        results = []
        for p in res['result']['stack']:
            results.append(ContractParameter.FromJson(p))

        self.assertEqual(len(results), 3)
        self.assertEqual(results[0].Value, bytearray(b'NEX Template V2'))
        self.assertEqual(results[1].Value, bytearray(b'NXT2'))
        self.assertEqual(results[2].Value, 8)

if you want to find the test_script you will need to look for the ScripBuilder in the Query method after getting the information of the contract:

from neo.Wallets.NEP5Token import NEP5Token
from neo.Implementations.Blockchains.LevelDB.LevelDBBlockchain import LevelDBBlockchain
import binascii

blockchain = LevelDBBlockchain("/home/dau/.neopython/docker-privnet/Chains/fixtures/test_chain")

Blockchain.RegisterBlockchain(blockchain)

contract = Blockchain.Default().GetContract("b9fbcff6e50fd381160b822207231233dd3c56c2")

hex_script = binascii.hexlify(contract.Code.Script)
token = NEP5Token(script=hex_script)

result = token.Query()

jseagrave21 commented 6 years ago

Wow. I spent a loong time trying to figure out how to do that! Good work @dauTT ! I am continuing to work on my remaining test but I have a feeling I am not going to be able to complete all of it. I will get as far as I can and then turn it over to you to look at.

dauTT commented 6 years ago

@jseagrave21 ,

I looked at the test case test_get_block_int in your branch:

def test_get_block_int(self):
        req = self._gen_rpc_req("getblock", params=[10, 1])
        mock_req = mock_request(json.dumps(req).encode("utf-8"))
        res = json.loads(self.app.home(mock_req))
        self.assertEqual(res['result']['index'], 9)
        self.assertEqual(res['result']['hash'], '0x7c5b4c8a70336bf68e8679be7c9a2a15f85c0f6d0e14389019dcc3edfab2bb4b')
        self.assertEqual(res['result']['confirmations'], 9598)
        self.assertEqual(res['result']['nextblockhash'], '0x7c5b4c8a70336bf68e8679be7c9a2a15f85c0f6d0e14389019dcc3edfab2bb4b')

I think this test case is not correct because we expect index = 10 instead of 9. However the test case pass why? The reason why it pass has nothing to do with #554. In general, at the present moment, when running our test cases by default it takes the setting of the TEST environment. So, the all privnet blockchain get the extra genesis block of TESTand therefore the chain is off of one. This is not correct. To fix this issue we need to activate the setting of privnet before we run the test cases. So please use this new update file going forward:

BlockchainFixtureTestCase_Dau.py.txt

Fortunately this change should not impact our previous testing. So, the new update test_get_block_int will look like this:

def test_get_block_int(self):
        req = self._gen_rpc_req("getblock", params=[10, 1])
        mock_req = mock_request(json.dumps(req).encode("utf-8"))
        res = json.loads(self.app.home(mock_req))
        print(res)
        self.assertEqual(res['result']['index'], 10)
        self.assertEqual(res['result']['hash'], '0xd69e7a1f62225a35fed91ca578f33447d93fa0fd2b2f662b957e19c38c1dab1e')
        self.assertEqual(res['result']['confirmations'], 12201)
        self.assertEqual(res['result']['nextblockhash'], '0x2b1c78633dae7ab81f64362e0828153079a17b018d779d0406491f84c27b086f')

In addition, I notice that the following assertion is no useful for us anymore:

self.assertEqual(res['result']['confirmations'], 12201)

If we look at the method, get_tx_output, behind the confirmations attribute we see that it is calculated as follow:

jsn['confirmations'] = Blockchain.Default().Height - header.Index + 1

This is not a good check in my opinion because every time we add a new block to the privnet fixtures, this confirmations number will change and therefore the assertion will fail. Perhaps a better replacement of the test case will be something like this:

self.assertEqual(res['result']['confirmations'], GetBlockchain().Height - 10 + 1)

Did you agree?

jseagrave21 commented 6 years ago

@dauTT Perfect! That answers my question exactly. I am glad they were unrelated issues! I am still working but I haven't had much time this week because of school. Let me know if you decide to handle the remaining portion of test_json_rpc_api.py yourself.

dauTT commented 6 years ago

@jseagrave21 no worry. I understand as I have also limited capacity due to work. So keep your pace. 👍

jseagrave21 commented 6 years ago

@dauTT For test_account_state_not_existing_yet, I need an empty wallet. Is there an empty wallet on your testcase?

dauTT commented 6 years ago

@jseagrave21 ,

the existing test case seems to work because this account, AHozf8x8GmyLnNv8ikQcPKgRHQTbFi46u2 does not exist on the privnet image (dautt/neo-privnet-unittest:v0.0.18)

def test_account_state_not_existing_yet(self):
        addr_str = 'AHozf8x8GmyLnNv8ikQcPKgRHQTbFi46u2'
        req = self._gen_rpc_req("getaccountstate", params=[addr_str])
        mock_req = mock_request(json.dumps(req).encode("utf-8"))
        res = json.loads(self.app.home(mock_req))
        self.assertEqual(res['result']['balances'], {})
        self.assertEqual(res['result']['script_hash'], addr_str)

So we don't need to create an empty wallet.

jseagrave21 commented 6 years ago

@dauTT okay, I didn't know if that accomplished the intent of the test. Thank you for looking at it!

ixje commented 6 years ago

I see so much effort and perseverance, keep it up! 💪 If there's anything I can do or clarify let me know. I might be a slow in responding as my online time is a bit on and off the last week and probably next week.

jseagrave21 commented 6 years ago

@dauTT I think I have migrated as much as I can figure out. Here is my latest test: https://github.com/jseagrave21/neo-python/blob/patch-5/neo/api/JSONRPC/test_json_rpc_api.py

Here are the failures/error I am still getting:

======================================================================
ERROR: test_send_raw_tx (neo.api.JSONRPC.test_json_rpc_api.JsonRpcApiTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/users/jseag/neo/neo-python-travis/neo/api/JSONRPC/test_json_rpc_api.py", line 425, in test_send_raw_tx
    self.assertEqual(res['result'], False)
KeyError: 'result'

======================================================================
FAIL: test_get_storage_item (neo.api.JSONRPC.test_json_rpc_api.JsonRpcApiTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/users/jseag/neo/neo-python-travis/neo/api/JSONRPC/test_json_rpc_api.py", line 342, in test_get_storage_item
    self.assertEqual(res['result'], '00f938dbba05')
AssertionError: None != '00f938dbba05'

======================================================================
FAIL: test_get_storage_item2 (neo.api.JSONRPC.test_json_rpc_api.JsonRpcApiTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/users/jseag/neo/neo-python-travis/neo/api/JSONRPC/test_json_rpc_api.py", line 352, in test_get_storage_item2
    self.assertEqual(res['result'], '003a0d7914b200')
AssertionError: None != '003a0d7914b200'

======================================================================
FAIL: test_get_storage_item_bad_contract_hash (neo.api.JSONRPC.test_json_rpc_api.JsonRpcApiTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/users/jseag/neo/neo-python-travis/neo/api/JSONRPC/test_json_rpc_api.py", line 376, in test_get_storage_item_bad_contract_hash
    self.assertTrue('error' in res)
AssertionError: False is not true

----------------------------------------------------------------------
Ran 48 tests in 9.474s

FAILED (failures=3, errors=1)

I am not sure how to pull out the required info from your contract. And, I don't know how to send a valid raw tx. cc @ixje

CityOfZion / neo-python

Reduce test fixtures size to speed up tests #478

Current behavior