Integration tests

[nivida]

Description

The Web3.js library QA should get improved with integration tests. The goal is to test it against several node types (Geth, Parity, and Ganache).
These integration tests should run in a docker image for later adding it to the GitHub actions.

Expected behavior

I can execute npm run e2e and it will test the whole library with several node types.

Actual behavior

There are no integration tests.

[szerintedmi]

I'm more than happy to chip in with a few basic ganache tests.

Where these test should sit in the repo structure?

I would just launch a container with a given version of ganache for now - later travis matrix or whatever magic can be added to test against beta etc. versions.

Some mock contracts might be required to test methods , events etc. To keep things simple I would just add those contracts via some pre-test scripts. Later we can consider creating a web3js test container derived from the standard ganache image, already preloaded with those.

I would launch ganache with a deterministic mnemonic so that it's easier to test signing functions.

@micahriggan : AFAIK you started to work on this. where are you with it?

[nivida]

@szerintedmi Sounds great! But I would probably use Geth for now because it's the most used node from the EF and is more up to date with the JSON-RPC spec.

[szerintedmi]

I didn't suggest not to test against Geth. I meant I'm happy to contribute with ganache integration tests because that's our current pain point with web3.
Although the same tests should run against geth/ganache/parity , only the container would be different so it shouldn't be a big deal to cover them.

[princesinha19]

Yes, @nivida is correct. Ganache doesn't support many RPC methods which are supported by Geth and Parity. So, for now, we should consider Geth.
For eg. ConsenSys-archive/ganache#408, ConsenSys-archive/ganache#405 etc.

I know ganache is used by most of the developers as a TestRPC. I also use ganache, but it's not up to date.

[szerintedmi]

Again, the same test should run against Geth/Parity/Ganache. So it's only a question which docker image you are running your tests against. Geth is not so important for us atm but it's literally should be one command to launch a parity or geth container. The heavy lifting here is to create proper tests.

Anyway, if ganache is not supporting something then integration tests are a great to expose it. Also running it against future ganache releases will show if they implemented it as you expected.

We use ganache for our on-chain tests. It used to have a lot issues but they iterated a lot and it became very stable and fast.

(Btw I think ConsenSys-archive/ganache#408 has been fixed. I commented there. Great example which would have been obvious with integration tests )

[sshelton76]

What about modifying this to test against any and all supplied RPC endpoints? Then we only need the one container + one parameter, maybe two parameters if there is login info required. It's going to be easier to maintain and those who want ganache or other older tech can see where those projects are falling behind, plus they can test vs services such as AWS blockchain in box and even Infura.

e.g.

npm run e2e https://rinkeby.infura.io/$ID

[szerintedmi]

npm run e2e https://rinkeby.infura.io/$ID

@sshelton76 : I think it's a great idea. Wondering if one or two param will be enough as each provider might require some options to run . but I guess we will see.

Would be great to test against Infura too. Do you have a thought how could we test transactions there? Do they have any mock or we would need to deploy mock contracts to a test network? That sounds a bit complicated for me...

I would propose one step a time and iterate. For me ganache tests are the easiest to start with as I have the most experience with that. Maybe I draft a very rudimentary first pass PR then we can discuss that?

(although first I have to make it work locally, npm test fails. looking into it but let me know if you aware of anything obvious I might do incorrectly)
UPDATE: test are running now, I just missed npm build :)
btw, npm install is not listed in the README, might be obvious but I thought npm bootstrap script does it.

FAIL tests/src/IbanTest.js
  ● Test suite failed to run

    Cannot find module 'web3-utils' from 'IbanTest.js'

      4 | 
      5 | // Mocks
    > 6 | jest.mock('web3-utils');
        |      ^
      7 | 
      8 | /**
      9 |  * Iban test

      at Resolver.resolveModule (../../node_modules/jest-resolve/build/index.js:229:17)
      at Object.mock (tests/src/IbanTest.js:6:6)

[levino]

Too general, not actionable.

[szerintedmi]

Too general, not actionable.

Indeed, it's an epic. I'm drafting a rudimentary PR as a conversation starter about approach.
Will push WIP soon, just trying to get out from Lerna and Jest rabbit holes.

UPDATE: great syncing of effort - just seen @micahriggan 's #2688 :))

[szerintedmi]

Lol, 3 of us :)
I won't push my PR, it doesn't add much to #2688 and #2693 and those are more than enough to start discussion.
I will add bits when you decided which PR to carry on with

[szerintedmi]

@nivida : I think you are pulling the trigger too soon by closing #2688 . It has been opened 7 hours ago. You are not leaving time for community review.

I'm still reviewing both PR-s and I wan't to try both locally too. And it's not about me, there are much more experienced people around who can't jump on immediately.

[nivida]

@szerintedmi Yeah, that's true but you were commenting there I should decide which PR should be the base. The decision was that I will take the PR from @levino. I think a simple base can be merged quickly and all further e2e tests have to be reviewed closer and from more than just one developer.