Possible E2E tests improvements

[NoriSte]

I was asked to analyze the Cypress end-to-end testing strategy optimizations.

I gained some experience in the field (you can find all my articles, courses, and talks here) and let me say one thing: they are very good tests 😊 I want to highlight this because most of the time I look at existing tests the situation is terrible, that's not the case here 😊

That said, there are a bunch of changes I would apply, but most of them are subjective.

Steps instead of tests

This is an example of the current test approach, where tests are used to document steps and all the tests continue the previous one.

it('contract file uploads', () => { // <-- a test used as a step
  assertUpload('erc20.contract');
});

it('moves to step 2', () => { // <-- a test used as a step
  assertMoveToStep2();
});

it(`types ${initialSupply} in the totalSupply field`, () => { // <-- a test used as a step
  cy.get('.form-field.totalSupply').find('input[type="number"]').eq(0).type(`${initialSupply}`);
  cy.get('[data-cy="next-btn"]').should('not.be.disabled');
});

this approach is unique (and also forced to put testIsolation: false, in the cypress.config.ts file since Cypress forces test isolation from version 12).

Despite there being no differences in terms of end result, tests should be atomic, and a single test should contain a full user flow. Anyway, my experience in the field allows me to highly appreciate the fact that every step includes a rich description! This eases debugging the tests a lot, and allows the tests to explain, from a user perspective, what they do!

How could we satisfy the general best practice and readability of the tests at the same time?

1. By using cypress-plugin-steps so the above code becomes something like

   
   cy.step('contract file uploads')
   assertUpload('erc20.contract');

cy.step('moves to step 2') assertMoveToStep2();

cy.step(types ${initialSupply} in the totalSupply field) cy.get('.form-field.totalSupply').find('input[type="number"]').eq(0).type(${initialSupply}); cy.get('[data-cy="next-btn"]').should('not.be.disabled');

2. Or by simply using a richer Cypress log, something like
```js
cy.log('**--- contract file uploads**')
assertUpload('erc20.contract');

cy.log('**--- moves to step 2**')
assertMoveToStep2();

cy.log(`**--- types ${initialSupply} in the totalSupply field**`)
cy.get('.form-field.totalSupply').find('input[type="number"]').eq(0).type(`${initialSupply}`);
cy.get('[data-cy="next-btn"]').should('not.be.disabled');

The former solution prints the steps also in the CLI (for cypress run) while the latter only reports the steps in the UI but avoids adding one more dependency.

Unclear selectors

In cases like the following one

cy.get('.form-field.to').find('.dropdown').click().find('.dropdown__option').eq(3).click();

I think a comment could be left there explaining what eq(3) refers to. Why? Because it does not tell the reader what it is selecting (what eq(3) refers to?). I'm thinking of the developers that have to understand what eq(3) was in the author's mind when the test is failing and they do not know anything about it. Something like (it's just an example)

// `eq(3) refers to the option to send to another account
cy.get('.form-field.to').find('.dropdown').click().find('.dropdown__option').eq(3).click();

It's not about selectors but the same could be applied here

cy.get('[data-cy="vector-add-2"]').click().click();

where it's not clear why a double click() is needed. Again, I'm thinking about the tests that break because of a change in the UI and the developers facing it could have a hard time understanding the reason behind the double click.

data-cy attributes in favor of classes

Despite it could result in more code, I would move the FormField from being selected through classes (cy.get('.form-field.spender')) to a data-cy approach that could avoid breaking the tests because of changes in the CSS classes. Anyway, I could miss some context here because I think that there could be good reasons for using the CSS classes...

Minor and subjective things

All the next feedback is more subjective, and they could be useful or not in given contexts or in case of some future evolutions of the project. They are not big things, but simply some feedback coming from my experience seeing what scales better.

data-testid instead of data-cy attributes

data-cy attributes are unique to Cypress, while data-testid are shared across all the testing frameworks. While data-cy could be used in other testing frameworks too, data-testid allow having more uniformity across different tests.

Testing Library instead of cy.contains

This falls again under the "more uniformity across tests" topic that could help in the future when the codebase and the type of tests will grow. In case of more tests, Testing Library selectors remain the same across all the tests and testing frameworks.

Remove as much as possible the dynamic parts of the tests

In my experience, every small layer of abstraction adds more cognitive load when developers read the tests, improving the doubts and time needed to understand what the tests do (that's crucial especially when they fail). I refer to one example I found the codebase

const messages1 = ['new', 'newDefault', 'failedNew', 'echoAuction', 'revertOrTrap', 'debugLog'];

cy.get('[data-cy="message-docs"]').each((item, i, list) => {
  expect(list).to.have.length(6);
  expect(Cypress.$(item).text()).to.contain(messages1[i]);
});

Instead, I would do something dumber like

cy.get('[data-cy="message-docs"]').alias('message-docs')

cy.get('@message-docs').should('have.length', 6)
cy.get('@message-docs').eq(0).should('contain', 'new')
cy.get('@message-docs').eq(1).should('contain', 'newDefault')
cy.get('@message-docs').eq(2).should('contain', 'failedNew')
cy.get('@message-docs').eq(3).should('contain', 'echoAuction')
cy.get('@message-docs').eq(4).should('contain', 'revertOrTrap')
cy.get('@message-docs').eq(5).should('contain', 'debugLog')

I talked about the same topic in this From unreadable React Component Tests to simple, stupid ones article of mine.

Notes

I did not use any of the existing templates because this issue is not a Bug report, nor a Feature request, nor a Security vulnerability.