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

Implement a first basic scenario with Cypress for the dApp #1584

Closed
3 of 4 tasks
kelsos opened this issue May 28, 2020 · 25 comments
Closed
3 of 4 tasks

Implement a first basic scenario with Cypress for the dApp #1584

kelsos opened this issue May 28, 2020 · 25 comments
Assignees
Labels
8 dApp 📱 infrastructure 🚧 Tests, CI, and general project infrastructure test Tests related issues

Comments

@kelsos
Copy link
Contributor

kelsos commented May 28, 2020

Description

This issue is a followup to #1404.
In #1580 the basics are done and Cypress works with the integration image.
The next step for us is to cover the basic dApp functionality with tests.

Acceptance criteria

  • Have tests for basic actions in the dApp
  • Runs in the CI
  • open channels
  • deposit to UDC
  • make a direct transfer
  • request a route
  • make a mediated transfer

Tasks

Additional information

@kelsos kelsos added dApp 📱 test Tests related issues infrastructure 🚧 Tests, CI, and general project infrastructure labels May 28, 2020
@kelsos kelsos added this to the Product Backlog milestone May 28, 2020
@christianbrb
Copy link
Contributor

Start with the basic main scenarios

@taleldayekh
Copy link
Contributor

I guess we need to discuss what the basic scenarios are and how to organise the tests.

@christianbrb christianbrb changed the title Introduce Cypress tests for dApp functionality Implement a first basic scenario with Cypress for the dApp Sep 30, 2020
@taleldayekh taleldayekh added the 8 label Sep 30, 2020
@taleldayekh taleldayekh removed this from the Product Backlog milestone Sep 30, 2020
@weilbith
Copy link
Contributor

weilbith commented Oct 1, 2020

@taleldayekh you assigned me as well. What tasks do you expect I should start to work on?

@taleldayekh
Copy link
Contributor

@weilbith I assigned you because I guess we will be having some discussions on how to best implement this stuff. :) But if you want you could resarch the CI integration.

@weilbith
Copy link
Contributor

weilbith commented Oct 1, 2020

Sounds good. Ping me when you have something ready I could try to make work with CI.

@christianbrb
Copy link
Contributor

christianbrb commented Oct 2, 2020

I was thinking about implementing the following scenario, but we would need to discuss how and if we break it down.

The scenario has been build under the assumption, that those are high level integration tests and not pure UI tests.

The big question: Should we run this end to end or split the tests?

I am not quite sure. Reading the cypress best practice docs:

It is common for tests in Cypress to issue 30+ commands. Because nearly every command has a default assertion (and can therefore fail), even by limiting your assertions you’re not saving yourself anything because any single command could implicitly fail.

Watching the video:

grafik

They also do not recommend to do E2E tests with Cypress, but they also say:
grafik

Some further reading: https://github.com/NoriSte/ui-testing-best-practices

@taleldayekh
Copy link
Contributor

@christianbrb Nice, thanks for the good job and effort in thinking that through.

@kelsos
Copy link
Contributor Author

kelsos commented Oct 2, 2020

In the case of the light client, it is harder to have pure UI tests since you would have to mock the SDK. If the integration image is used as a base I don't think that it would be a problem having e2e tests in Cypress, since everything happens in a controlled environment.

@weilbith
Copy link
Contributor

The Dockerfile for the e2e tests is huge. Therefore I'm asking you @kelsos in the meantime. I don't understand how the following is possible: I start a new container based on this image. I test the light client and do some transactions etc. I stop and remove the container again. I restart that procedure. Now I can observe that the old blockchain data is still there. I take the oncounting block number as proof that the data is still there. How is that possible, if there is no volume attached and I take the same base image? Is there something stored else on my host system? I'm very confused...

@kelsos
Copy link
Contributor Author

kelsos commented Oct 13, 2020

By old blockchain data do you mean that the block number is not zero and there are some data there?. If that is the case then that is how things are supposed to be. The block number should not be zero. When the image is built, the Raiden contracts are deployed and a channel is opened between two Python Raiden nodes. Geth's state is then stored in the new image.

Each new container when started should have the contracts deployed and two python Raiden nodes with an open fully funded channel between them. This happens during the image creation and it is part of the image. The reason behind this is that we wanted to have a faster execution time when running the tests. Deploying the contracts and opening the channels takes time and it is not what we want to tests. That is one of the reasons the image is more complicated and takes time to build, however, it is way faster on runtime.

Does this help?

@weilbith
Copy link
Contributor

Does this help?

No, unfortunately not. That is what we know.
Let's say that after the image is built, the Geth node is at block number 200. Then we start a container based on this image. We connect the light-client to it and use it a bit. Now we are at block 230. There are open channels written to the state of the contract etc. Now stop and remove the container. Start a new one based on the same image as before.
What I would expect: I'm again at block 200, no contract state (except of the deployment).
Actually: Geth is now at block 230 and continues. The contracts state is still there.

I must admit that I technically don't understand this issue. There must be something persistent. But I don't know how this is possible due to the cgroup fo the container.

@kelsos
Copy link
Contributor Author

kelsos commented Oct 13, 2020

Do you properly delete the previous container? I remember always stopping, then deleting the container and then creating a new one.

@kelsos
Copy link
Contributor Author

kelsos commented Oct 13, 2020

I am probably asking stupid things, let me check the previous code and changes a bit maybe I can get an idea

@weilbith
Copy link
Contributor

@kelsos Don't waste your time. I can do this in the time I'm getting payed for. I just thought asking would be helpful.

@kelsos
Copy link
Contributor Author

kelsos commented Oct 13, 2020

@weilbith ok no worries. By checking I can see that you use the --rm flag now with docker so the image is supposed to be deleted when the execution terminates. Any change that the image is not getting deleted for some reason? maybe it doesn't stop properly so rm fails?

@weilbith
Copy link
Contributor

I'm 100% sure container is removed.
I do now a actual analysis of this. I try to reproduce what Talel saw in the light client with pure RPC requests and manual docker control.

@kelsos
Copy link
Contributor Author

kelsos commented Oct 13, 2020

I'm 100% sure container is removed.
I do now a actual analysis of this. I try to reproduce what Talel saw in the light client with pure RPC requests and manual docker control.

Ok, but in general giving a quick look at the changes, there is no reason why the state would persist when the image is removed. It didn't before and I didn't see any change in the Dockerfile that would change that.

@weilbith
Copy link
Contributor

ist when the image is removed

Wait what? Why should I remove the image? Only the container based on the image should get removed. Then a new container gets started based on this image.


Anyways. I could not reproduce the issue with the following test:

  • start container manually
  • query block number via direct RPC requests (ongoing number from ~230)
  • stop and remove the container manually
  • start the container manually again
  • query block number via direct RPC requests

-> They start again from the block after the image build was done and all contracts deployed.

This result is reproducable. The image is fine (happily, else I would have been super confused). So the issue must occur somewhere later in the setup. The debugging in the Cypress electron app was kinda clunky. So I don't trust these results anymore. Eventually it is just that Cypress cleans localstorage, but not the indexdb and then it looks like that.
So I continue to test it step by step.

@kelsos
Copy link
Contributor Author

kelsos commented Oct 13, 2020

Sorry I meant container

@weilbith
Copy link
Contributor

Thanks for your help. Was a nice feeling to have you on board for some minutes. 🤗

@weilbith
Copy link
Contributor

Just for the protocol: what affected my confused mind was that Talel stated that if he rebuilt the image from scratch (apparently some layer caching is not working) this was working fine again. So I first thought the issue can't by at Cypress then...

@weilbith
Copy link
Contributor

weilbith commented Oct 13, 2020

But eventually the re-build just causes the new persistent layer of the dApp to create a new DB. Talel his electron app was pretty cluttered already.

Since also the script works fine (replaced the run of Cypress with two times quering the block number via RPC), it must be something with Cypress and how the dApp parses the old state in contrast to what the Eth node says.

@weilbith
Copy link
Contributor

Quick update: Talel told me that actually applying the manual delete of the database in the support index script is doing the job. Here is the issue of Cypress discussing the problem and providing the suggestion.

@quantuminformation
Copy link

Good luck with that, I've been fighting it all day )

@christianbrb
Copy link
Contributor

The Cypress tests have been implemented and @taleldayekh has shown the progress on his local machine.

Closing this issue to get better organized to talk about it in more detail within the planning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8 dApp 📱 infrastructure 🚧 Tests, CI, and general project infrastructure test Tests related issues
Projects
None yet
Development

No branches or pull requests

5 participants