5afe / safe-react

Deprecated! New repo – https://github.com/safe-global/web-core
MIT License
332 stars 363 forks source link

Testing Suite Automation #559

Closed pablofullana closed 4 years ago

francovenica commented 4 years ago

Current repo: https://github.com/francovenica/AutomGnosisSafe

francovenica commented 4 years ago

Some info about how this went through: Puppeteer was selected to do this since there is a library that extends puppeteer functionality called dappeteer, that gives methods to load metamask in a browser, connect to it, import accounts, sign transactions... Also Jest was integrated so we can have actual results after a test (how long it took, which failed, assertion methods, etc) At some point we tried Cypress that works better in general and is more user friendly, but loading extensions into it was hard and since you cannot work with more than 1 tab at the time there we had to discard it

It works well, it does what it's supposed to do. So far we were able to load metamask into the browser, connect to the application, and create a safe from scratch by signing the tx in metamaks. If all that was possible then it looks like the rest of the test we do manually can also be automated

The head of master is always a stable version that should be run well enough, but take in account that the code might be rough, unpolished and over all not that pretty. I'm trying to make it work first and then comes the optimization. Kudos to Fernando and the rest of the guys for giving me a hand with that.

Drawbacks: There are things that don't work well yet and require better implementation or more research with the tool. Also other things that sadly seem impossible to do: 1 - Is susceptible to failure because of low speed connection or Metamask taking a long time to load: There is simply no way around, if page takes too long to load then it will fail at some point. The only solution to this seems to be giving to every test a time out of several minutes. 2 - Seems to be impossible to do it with a headless browser: Since the browser has to load the metamask extension the browser has to be present, so it cannot be integrated to a CI server for now. I've tried with "Brave" since that browser has the wallet integrated (no need for the load of the extension in the same way chrome does) but it doesn't work, it seems Brave has compatibility issues. 3 - Every test has to be independent: I tried to connect to the wallet in the 1st test and then the rest of the test just keep using the open browser with the connection established, but for some reason it seems that the variables that hold the session open become undefined as you jump to another file, so right now every test has to open the browser and import the full wallet over and over again. Is not a big time loss but it might get annoying

francovenica commented 4 years ago

I'm done with a test to send funds. Master was updated already.

@pablofullana what should we do with this ticket? we should keep it open and be renewed in every sprint? or can we close it here and open new ones as they are needed?

Right now my status with automation is: keep automating the test cases I have written and I do manually for every release until there is a specific task that needs to be automated by request of the people from Gnosis.

francovenica commented 4 years ago

I'm done with 2 more tests: -modify policies -add/remove owners

both were updated in the master repo

Now I'm working with "replacing owners" but there are 2 tickets that are messing up that test: https://github.com/gnosis/safe-react/issues/635 https://github.com/gnosis/safe-react/issues/649

I can still work on the test, but I'll be not able to complete it until those issues are fixed

lukasschor commented 4 years ago

This seems like a ticket that would never be closed as this seems to be an ongoing task. Should we just close this and open tickets for individual tests that should be added in the future? Or do I not understand the purpose of this ticket correctly? @francovenica

francovenica commented 4 years ago

@lukasschor Yes we should close it.

pablofullana commented 4 years ago

hey @lukasschor. as far as I can recall, this issue was intended to track the testing suite automation setup. Just the setup, with a few tests to validarte that it is working as expected. So I guess it would be better to close this one and open specific issues for different sets of tests.

pablofullana commented 4 years ago

NVM, too late with my comment :)

lukasschor commented 4 years ago

But good that we are aligned :)