Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parity Integration Testing #381

Closed
carver opened this issue Oct 24, 2017 · 17 comments
Closed

Parity Integration Testing #381

carver opened this issue Oct 24, 2017 · 17 comments

Comments

@carver
Copy link
Collaborator

carver commented Oct 24, 2017

  • Version: 4.*
  • Python: 3.5

What was wrong?

We don't have any integration tests for parity.

It's very difficult to test code like #366

How can it be fixed?

Generate some data in a parity fixture database (similar to how the geth one works) and build the integration conftest file.

@carver carver added this to the Version 4 Stable milestone Oct 24, 2017
@pipermerriam
Copy link
Member

We also need some way to install a specific version of parity which should likely be done with a small shell script to start off with until we can iterate towards a generic solution.

@carver
Copy link
Collaborator Author

carver commented Oct 24, 2017

Great point. For determining which versions to test against, I could see which versions are used in the nodes on https://ethstats.net/

Any other ideas? How did you decide for geth?

@pipermerriam
Copy link
Member

For determining which versions to test against

I would suggest only the latest point releases excluding any with known bugs, starting with whatever the current latest is.

How did you decide for geth?

Whatever was newest at the time.

@carver
Copy link
Collaborator Author

carver commented Jan 12, 2018

@dylanjw here's the parity integration test issue.

Here's an example integration "setup" file: https://github.com/ethereum/web3.py/blob/master/tests/integration/test_goethereum.py

It ultimately calls all these tests against the fixtures for geth: https://github.com/ethereum/web3.py/tree/master/web3/utils/module_testing

For example: https://github.com/ethereum/web3.py/blob/master/web3/utils/module_testing/eth_module.py


So one way to set up the parity integration tests would be to:

  1. Generate a blockchain with some specific values, like a new funded account. The geth one is here: https://github.com/ethereum/web3.py/blob/master/tests/generate_go_ethereum_fixture.py
  2. Generate fixtures that refer to those values. like a funded_account_for_raw_txn fixture that returns the hex address of the account that's funded.
  3. Add a method to download/install a specific parity version (maybe a py-parity, equivalent to py-geth)
  4. Add a tox environment in tox.ini with the parity setup
  5. Add a travis test run into .travis.yml for each parity version to test

I didn't set up the geth integration tests, so it's totally possible that I'm missing some things in this high-level approach, but the above steps should be a good place to start.

@pipermerriam
Copy link
Member

pipermerriam commented Jan 12, 2018

See here for documentation on the necessary fixtures for the intergration tests: https://github.com/ethereum/web3.py/blob/master/tests/integration/README.md

@dylanjw
Copy link
Contributor

dylanjw commented Jan 31, 2018

I have created fixtures for parity. Because parity does not have mining functionality, I used the following method to in the generation script:

  • Start a geth node on a private network, and run the fixture generation script (adapted from the existing script)
  • Start a parity node and peer it with the geth node.
  • Wait for sync to complete.

A big challenge was creating matching genesis files. Parity requires that you configure the starting block number for various EIPs. Some of these are configurable in the geth genesis file and others are not.

The majority of the tests that depend on the fixtures are currently failing. Tomorrow I will dig into the tests themselves and do some clean up of the fixture generation scripts.

@carver
Copy link
Collaborator Author

carver commented Feb 1, 2018

The majority of the tests that depend on the fixtures are currently failing.

Feel free to post a list of the failing tests, along with the ones you think make sense to focus on, and others that make sense to punt.

@5chdn
Copy link

5chdn commented Feb 1, 2018

Because parity does not have mining functionality

Note, Parity does not have a mining functionality for Ethash, but it provides other engines of sealing blogs for AuRa, Tendermint, "Instant Seal" (Dev mode).

If a PoW environment causes you headache, you can just switch the engine.

@dylanjw
Copy link
Contributor

dylanjw commented Feb 1, 2018

Thanks @5chdn, I'll take a look at the other engines.

@dylanjw
Copy link
Contributor

dylanjw commented Feb 1, 2018

@carver Running tests on my system (Im still working on the travis CI config), I get 10 failed, 87 passed.

Here is a summary of the failing tests:

  1. TestParityEthModule.test_eth_newBlockFilter
    AssertionError on line 438
  2. TestParityEthModule.test_eth_uninstallFilter
    AssertionError on line 544
  3. TestParityPersonalModule.test_personal_importRawKey
    MethodNotFound: personal_importRawKey
  4. TestParityPersonalModule.test_personal_listAccounts
    MethodNotFound: personal_listAccounts
  5. TestParityPersonalModule.test_personal_lockAccount
    MethodNotFound: personal_lockAccount
  6. TestParityPersonalModule.test_personal_unlockAccount_success
    MethodNotFound: personal_unlockAccount
  7. TestParityPersonalModule.test_personal_unlockAccount_failure
    MethodNotFound: personal_unlockAccount
  8. TestParityPersonalModule.test_personal_newAccount
    MethodNotFound: personal_newAccount
  9. TestParityPersonalModule.test_personal_sendTransaction
    MethodNotFound: personal_sendTransaction
  10. TestParityPersonalModule.test_personal_sign_and_ecrecover
    MethodNotFound: personal_sign

Im going to start with looking into 1 & 2. For the failures due to missing methods, I will probably drop all of these. I will also look at adding a parity module and tests.

@carver
Copy link
Collaborator Author

carver commented Feb 1, 2018

Im going to start with looking into 1 & 2. For the failures due to missing methods, I will probably drop all of these.

Sounds totally reasonable. Looking pretty close!

One approach to dropping the personal_* tests would be:

class TestParityPersonalModule...
    def test_personal_listAccounts(self):
        pytest.xfail('this non-standard json-rpc method is not implemented on parity')`

Plus, adding a xfail_strict=true line to the pytest.ini (which has been on my todo list).

Then we get notified if any of them start working.

@dylanjw
Copy link
Contributor

dylanjw commented Feb 1, 2018

I will add TestParityEthModule.test_eth_uninstallFilter to the xfail tests. Parity returns true for any filter id parameter I enter, so I raised this issue: openethereum/parity-ethereum#7783

@dylanjw
Copy link
Contributor

dylanjw commented Feb 1, 2018

For the test_eth_newBlockFilter failure, on the first poll parity returns the latest block hash, instead of none. Subsequent polls return the expected empty list

    def test_eth_newBlockFilter(self, web3):
        filter = web3.eth.filter('latest')
        assert is_string(filter.filter_id)
    
        changes = web3.eth.getFilterChanges(filter.filter_id)
        assert is_list_like(changes)
>       assert not changes
E       AssertionError
changes    = [HexBytes('0xa449e487af78da9478c2e8e3a435a4d4a08fa5d8891d79c80abc3f1c3f6c1b93')]

(Pdb) web3.eth.getFilterChanges(filter.filter_id)
[]

@dylanjw
Copy link
Contributor

dylanjw commented Feb 1, 2018

I marked test_eth_newBlockFilter as xfail as well.

@carver
Copy link
Collaborator Author

carver commented Feb 1, 2018

Parity returns true for any filter id parameter I enter, so I raised this issue: openethereum/parity-ethereum#7783

Awesome 👍

on the first poll parity returns the latest block hash, instead of none

Interesting find. Did you file an issue for that as well?

@dylanjw
Copy link
Contributor

dylanjw commented Feb 2, 2018

Interesting find. Did you file an issue for that as well?

I did not. Will do.

@dylanjw
Copy link
Contributor

dylanjw commented Feb 5, 2018

openethereum/parity-ethereum#7816

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

No branches or pull requests

4 participants