HathorNetwork / hathor-wallet

Hathor Official Wallet for Desktop
https://hathor.network/
MIT License
33 stars 24 forks source link

[Design] Automated tests #357

Open tuliomir opened 1 year ago

tuliomir commented 1 year ago

Motivation

We currently spend many hours from the dev team running a QA process when we need to release a new version of the desktop wallet, or just validate that the wallet is working fine after a big refactor/change in the code.

Summary

We must have an easy way to automatically test the whole desktop wallet in order to decrease the time spent in QA's. The goal for this project is to create a design that will cover the full automated test cycle.

Acceptance Criteria

Guide-level description

Unit testing

Since we used the create-react-app helper to create this application, we already use jest as a test framework.

Following the recommendation from the main docs we will also use the react-testing-library as a set of testing helper APIs. It allows the test to easily:

On the existing components' side, for a better testing procedure it's important to export each of them apart from its redux-connected default export, as shown on the WalletAddress component on the PoC ( commit diff ).

// This class was not exported before. Should be for testing
export class WalletAddress extents React.Component {
  ...
}

// Previously existing export, used integrated on the application
export default connect(mapStateToProps, mapDispatchToProps)(WalletAddress);

End-to-end testing

The end-to-end testing is a way to run the application itself and interact with it simulating the user inputs. At the moment, the tests will be executed on the testnet, and as such the first interactions should set the server accordingly.

The selected framework was Cypress, as it is one of the most popular and well-documented, because of its ease of implementation and use. The PoC has some bootstrap code to allow for a faster implementation later.

The testing scope for this design is the interaction with the Software Wallet only. Considerations about the Hardware Wallet testing are offered on the Future Possibilites section.

Funding of local test wallet

Since many of the tests require starting with an empty wallet and then adding tokens to it, it would be necessary to have a source for those funds. They will have to be transferred programatically at test run time and then used on the tests themselves.

To avoid unnecessary consumption of HTR tokens, all tests should, at the end, melt every custom token generated to ensure the original HTR balance will be preserved - exception made for the NFTs that have a necessary fee. Those funds should finally be sent back to the source wallet to provide for future tests.

It also would make it easier to change the values on some of the current QA tests to a fraction of what they are, so that fewer tokens are needed for each test iteration. These changes would make it harder for a human tester to identify the results, but is trivial for a machine-run test suite.

An alert should be raised at the alerts-manager whenever there are no funds available for the tests, so that the current on-call can provide the funds and the tests can be re-run.

Sending funds for tests To send the funds, the ideal way is to import an existing wallet on the desktop wallet itself, using the Cypress interactions to do so. The wallet seed can be stored on a environment variable so that:

Once initialized, the test context itself will know the destination address to fund the newly created test wallet. After all the tests, the funds can be returned to the same source address.

PoC code

Available on PR #365

Rationale and Alternatives

Alternative framework: Puppeteer

At the moment, the main UI test frameworks available for javascript web apps are Puppeteer and Cypress.

Puppeteer is recognized as the most focused on programmatic testing — fitting for our GitHub CI workflow — while Cypress has more features focusing the interaction between Quality Assurance professionals and developers and some issues with breakpoint debugging.

After trying to implement it, some problems were found with the react-scripts dependency we currently have and this framework was discarded. Below are the steps taken and errors found while doing it.

Manual script

A manual script to ensure Puppeteer interaction with the app was implemented: running the application using npm run start and then executing the script below shows that Puppeteer can connect with the application and read its state successfully.

const puppeteer = require('puppeteer');  

(async () => {  
  const browser = await puppeteer.launch();  
  const page = await browser.newPage();  

  await page.goto("http://localhost:3000");  
  const selector = '#welcomeTitle'  
  await page.waitForSelector(selector)  
  const text = await page.$eval(selector, (e) => e.textContent);  

  // This next section would be replaced by a simple "expect(text).toContain('Welcome');"
  console.log(text);
  if (text.indexOf('Welcome') < 0) {
    throw new Error('Assertion failed')
  }

  await browser.close();  
})();

This can be done by simply running npm install puppeteer --save-dev before the script execution.

However, once Jest is involved it inserts its own module resolver on the runtime flow. This breaks the interaction between the puppeteer module and its underlying lib puppeteer-core. This is a known issue described on the Troubleshooting Guide.

The errors found had variations of:

TypeError: this._environment.runScript is not a function
Cannot find module 'puppeteer-core/.../xxxx.js'

Troubleshooting Jest + Puppeteer

The solution offered was to update Jest, but the wallet desktop currently has no direct integration with Jest: it interacts with it only through the react-scripts lib. Updating it would require potentially a development effort that would not fit inside a proof of concept scope.

Tentatives were made to:

As these paths failed, the amount of dependencies increased and all paths pointed to upgrading the jest version to v5, which is known to add more complex and application-wide necessary changes. So this path was discarded.

Possible paths for this framework would be:

Future possibilities

HTTP Interception and mocking

The main idea of the E2E tests are to validate the behavior of the application on a live network, so mocks and third-party call interceptions should be avoided whenever possible.

However, some tests may benefit from mocking, such as backend failure simulations. In these cases Cypress has a useful intercept tool that can both intercept the call and add a previously registered fixture as the response for that method.

Ledger interaction

To test the interaction with a hardware wallet it's necessary to use an emulated device. The official Ledger Portal offers the following resources:

The current implementation of the desktop application, however, does not allow interaction with this emulated device because of the hardcoded transport layer package used (@ledgerhq/hw-transport-node-hid). A transport package should be used to communicate with the device simulator but it still needs an update by the Wallet Desktop's side, which would require another project dedicated to it.

Task Breakdown

Unit testing done in 30,4 dev days on two main sections: Components and Screens E2E tests partially implementing the current QA process done in 13,1 dev days Total: 43,5 dev days

Components

Components folder testing: 18,9 dev days Components Effort Comments
BackButton 0,1
ChoosePassword 0,1
ChoosePin 0,1
HathorAlert 0,4 timer intervals, external direct call to method
HathorPaginate 0,1 mocks a complex child, but little local logic
InputNumber 0,5 many events, “forwardRef” testing
InputsWrapper 0,1
LedgerSignTokenInfo 0,1
Loading 0,2 many parameters to test
ModalAddManyTokens 1,0 many elements to validate, complex conditional flows
ModalAddressQRCode 0,5 redux connection, timer, clipboard, mocked download
ModalAddToken 0,4 token validation mocks, complex conditional rendering
ModalAlert 0,2 many parameters to test
ModalAlertNotSupported 0,1
ModalBackupWords 1,5 many flows to validate
ModalConfirm 0,3 external direct call to method affects rendering
ModalEditToken 0 unused, should be deleted
ModalLedgerResetTokenSignatures 0,5 complex flow, mocks and uses new “useEffect” syntax
ModalResetAllData 0,3 Validation flows depent on user input
ModalSendTx 0,7 rendering conditioned to saga events, complex flow
ModalUnhandledError 0,2 Needs to mock sentry and some handlers
ModalUnregisteredTokenInfo 0,3 list validation and settings conditions
Navigation 0 No testing, just a structure wrapper
NFTListElement 0,3 video and audio element to mock and test
OutputsWrapper 1 many direct lib accesses to mock
PinInput 0,1
PinPasswordWrapper 0 tested on PoC
RequestError 0,4 needs to mock deprecated methods on lib
SendTokensOne 1,5 complex flow, linked to saga events, child mocks
SendTxHandler 0,3 needs to mock the lib’s SendTransaction class
ServerStatus 0,1
SoftwareWalletWarninrMessage 0 No logic, no need to test it
SpanFmt 0,2 medium rendering complexity logic
TokenAdministrative 1,5 full screen complexity to test
TokenBar 1,2 multiple rendering logics, interaction with state
TokenGeneralInfo 0,4 Need to mock interaction with modal
TokenHistory 1 pagination testing with many fields
TokenPagination 0,1 just a wrapper
TxData 2 full screen complexity to test
TxRow 0 unused, should be deleted
TxTextInput 0 unused, should be deleted
Version 0,1
Waitversion 0 unused, should be deleted
WalletAddresses 0,7 needs to mock lib methods, modal interaction
WalletBalance 0,2
WalletHistory 0,1

Screens

Screens folder testing: 11,5 dev days Screen Effort Comments
AddressList 0,3 mock of a generator function, pagination
ChoosePassphrase 0,4 many steps, timer and form validation
CreateNFT 1 many fields, saga interaction, modals
CreateToken 1
CustomTokens 0,2
LoadingAddresses 0,5 state changes can cause redirects
LoadWallet 0,4 many minor validations on change and clicks
LoadWalletFailed 0,1
LockedWallet 0,4 many minor validations
NewWallet 0,6 backup flow, intermediate states
NFTList 0,3 stateToProps has many validations
Page404 0,1
SendTokens 1,5 complex interactions, states and hardware wallet flow
SentryPermission 0,1
Server 0,5 event handler on modals, direct write to wallet lib store
Settings 0,4 reset and export features can be complex to test
SignIn 0,1
SoftwareWalletWarning 0,1
StartHardwareWallet 0,4 Interactions with ledger must be mocked
TransactionDetail 0,2 API queries and external link
UnknownTokens 0,4 direct lib method calling, direct manipulation of rendered elements
VersionError 0,1
Wallet 2 a hub for many children components interaction
WalletType 0,1
WalletVersionError 0,2
Welcome 0,1

QA Interactions

The objective here is to execute a series of steps as close as possible to the application's QA. Effort: 13.1 dev days

Task Effort Comment
Set fullnode to testnet 0.5 The first contact of the developer will be here
Initialization 1.5 Backing up the words will take some effort
New Token Error 0.2
Fund local wallet 1.5 Implement the retrieval of funds to the local test wallet
Wallet Screen 1.0 Should use a known wallet with funds, some effort to read addresses on screen
Lock / Unlock 0.1
Transaction Detail 0.2
Create New Token 0.4 Also make the details validation
Send Tokens 0.3 Many screen changes
Unregister TST 0.3 Also make the details validation
Register in the 'Custom Tokens' 1.0 Both the QA steps: token bar and tokens screen
Administrative Tools 1.5 Many small interactions in this step
Hide Zero-Balance Tokens 0.8
Always Show Tokens 1.2
Token Bar Scroll 0.5 A way to read the element height would simulate the necessary scroll
Change Server 0.5 Ensure that no transactions are executed on the other network
Add Passphrase 0.3
Notifications and Bug Report 0.1
Register Tokens with same name 0.4
Try to spend same output 0.4
Create NFT 0.4

Task breakdown: Proposed approach

Start by the low-hanging fruits to allow for PRs that are shorter, simpler and faster to implement, review and deploy.

In this rationale, the suggested first week would have 3,6 estimated dev days. Future deploys would build upon existing code infrastructure and be easier to follow the development flow.

2,2 dev days of Components Component Effort
BackButton 0,1
ChoosePassword 0,1
ChoosePin 0,1
HathorPaginate 0,1
InputsWrapper 0,1
LedgerSignTokenInfo 0,1
ModalAlertNotSupported 0,1
PinInput 0,1
ServerStatus 0,1
TokenPagination 0,1
Version 0,1
WalletHistory 0,1
Loading 0,2
ModalAlert 0,2
ModalUnhandledError 0,2
SpanFmt 0,2
WalletBalance 0,2
1,4 dev days of Screens Screen Effort
LoadWalletFailed 0,1
Page404 0,1
SentryPermission 0,1
SignIn 0,1
SoftwareWalletWarning 0,1
VersionError 0,1
WalletType 0,1
Welcome 0,1
CustomTokens 0,2
TransactionDetail 0,2
WalletVersionError 0,2
The second week could advance on the E2E tests, alleviating the load on the manual QA process: Task Effort Comment
Set fullnode to testnet 0.5 The first contact of the developer will be here
Initialization 1.5 Backing up the words will take some effort
New Token Error 0.2
Fund local wallet 1.5 Implement the retrieval of funds to the local test wallet
Send Tokens 0.3 Many screen changes
tuliomir commented 1 year ago

Reviewers: @pedroferreira1 , @alexruzenhack

alexruzenhack commented 1 year ago

Does the Puppeteer work with the Electron?

It is possible to test UI without Puppeteer, we can use React Testing Library + Jest. Did you hear about this library? With this combo, the components can be rendered and navigated by DOM API and then asserted.

alexruzenhack commented 1 year ago

It is possible to test UI without Puppeteer, we can use React Testing Library + Jest. Did you hear about this library? With this combo, the components can be rendered and navigated by DOM API and then asserted.

Now I'm reviewing the PoC and see you use the React Testing Library already. Nice!

pedroferreira1 commented 1 year ago

As such, possible paths for this UI test would be:

Update all react dependencies to use react-scripts@5 and try again

We've talked about trying to upgrade only the react-scripts and build a poc of one test suite using pupeteer, even if some parts of the app end up broken. Did you try to do it?

tuliomir commented 1 year ago

...we can use React Testing Library + Jest.

Added a Guide Level Description section documenting that these libs are being used on the PoC. :+1:

pedroferreira1 commented 1 year ago

To avoid unnecessary consumption of HTR tokens, all tests should, at the end, melt every custom token generated to ensure the original HTR balance will be preserved - exception made for the NFTs that have a necessary fee. Those funds should finally be sent back to the source wallet to provide for future tests.

Why don't we run the tests using the testnet, so we don't really need to worry about losing funds?

Also have you validated how to run those tests using ledger? I know we have an emulator but not sure if they can be connected.

tuliomir commented 1 year ago

...have you validated how to run those tests using ledger?

Added considerations about the hardware wallet / ledger testing: they will be done in a future moment and are described on the Future Possibilities section.


...we don't really need to worry about losing funds?

About the amount of funds, while we have a lot of funds available on the testnet, we also have to refill the funds on the source wallet manually. Since the tests are automated and may be executed very often, I think It's better to preserve these funds only to delay as much as possible the manual refilling process that will be done by the on-call.

What do you think?


Also, added a section on mocking and intercepting HTTP requests on the Future Possibilities section to cover a comment on the PoC PR.

pedroferreira1 commented 1 year ago

About the amount of funds, while we have a lot of funds available on the testnet, we also have to refill the funds on the source wallet manually. Since the tests are automated and may be executed very often, I think It's better to preserve these funds only to delay as much as possible the manual refilling process that will be done by the on-call.

What do you think?

Yeah, this is good, I just think we should spend much time on that, especially when you say

It also would make it easier to change the values on some of the current QA tests to a fraction of what they are, so that fewer tokens are needed for each test iteration.

When we create a token, we need to create at least 1.00 amount to be able to melt and get the HTR deposit back. And even returning the tokens, it's always better to use testnet token and this was not mentioned in the design, that's why I wrote about it.

And this design is approved for me