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

Invite to pull request Alegria #14

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ldasilvaNZ11, o .env.example deve sim ser versionado, visto que não contém dados sensíveis, é só um exemplo, como o nome indica, de qual estrutura deve ser usada no .env, esse sim não versionado.

Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,5 @@

# misc
.DS_Store
**.env
guilhermealegria marked this conversation as resolved.
Show resolved Hide resolved

15 changes: 15 additions & 0 deletions cypress.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const { defineConfig } = require("cypress");

module.exports = defineConfig({
e2e: {
env:{
grepFilterSpecs: true,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vejo que usou o @cypress/grep para a execução dos teste de GUI e API separadamente.

No entanto, uma maneira mais simples seria informar ao Cypress quais arquivos (ou pastas) seriam executados, usando a opção --spec.

Exemplo:

cypress run --spec 'cypress/e2e/gui/*.cy.js'

baseUrl: 'http://localhost:3001/'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A propriedade baseUrl deve apontar para a URL do frontend, não da API.

Para a URL da API, você pode definir uma env.

Exemplo:

env: {
  API_URL: 'http://localhost:3001'
}

E a baseUrl deve apontar para http://localhost:3000.

},
fixturesFolder: false,
setupNodeEvents(on, config) {
require('@cypress/grep/src/plugin')(config);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ao fazer o que sugeri acima, tal lib nem mesmo seria necessária, tornando o projeto mais simples, que é o que procuramos quando se fala de um bom design: simplicidade.

return config;
},
},
});
108 changes: 108 additions & 0 deletions cypress/e2e/apitest/customers_request.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/// <reference types="cypress" />

wlsf82 marked this conversation as resolved.
Show resolved Hide resolved
describe('All test intermediary API', {tags: 'api', env: {baseUrl: 'http://localhost:3001/customers'}}, () => {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essa linha tbm pode ser removida pela mesma razão anterior.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visto que você não implementou a sugestão, essa conversa não deveria ter sido resolvida.

context('Request all customers', () => {
beforeEach('Get request', () => {
cy.request(Cypress.env('baseUrl')).as('customers')
guilhermealegria marked this conversation as resolved.
Show resolved Hide resolved
guilhermealegria marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baseUrl não é uma env, portanto não é assim que acessa-se a mesma.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Veja o comentário anterior.

})
guilhermealegria marked this conversation as resolved.
Show resolved Hide resolved

it('Check status code return for all customers', () => {
cy.get('@customers').then(({ status }) =>{
expect(status).to.eq(200)
})
})
guilhermealegria marked this conversation as resolved.
Show resolved Hide resolved

it('Check length for return with all customers', () => {
cy.get('@customers').then(({ body }) =>{
expect(body.customers).to.length(10)
})
})
})

context('Request all customers with pagination', () => {
const pagination = [1,2,3,4,5,6,7]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De onde saíram estes números?

E se só tiverem 20 clientes no banco de dados? A partir da página 3 já não viria nada.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Faltou responder minha pergunta.

it.each(pagination)('Check pagination with all customers', (page) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pra que tal complexidade?

Veja abaixo uma solução simples e direta-ao-ponto.

it('paginates the customer list correctly', () => {
  cy.request('GET', `${CUSTOMERS_API_URL}?page=2`).as('getCustomersPageTwo')

  cy.get('@getCustomersPageTwo')
    .its('body.pageInfo.currentPage')
    .should('eq', 2) // Supposing there are at least 2 pages
})

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Não gostou da minha sugestão?

cy.request(`${Cypress.env('baseUrl')}?page=${page}`).should(({status, body}) => {
expect(status).to.eq(200)
expect(body.pageInfo.currentPage).to.eq(page)
})
})
})

context('Request with get limit customer', () =>{
const limit = [1,2,3,4,5,6,7,8,9,10]
it.each(limit)('Filter to customers with inside limit', (limit) => {
cy.request(`${Cypress.env('baseUrl')}?page=1&limit=${limit}`).then(({status, body}) => {
expect(status).to.eq(200)
expect(body.customers).to.length(limit)
})
})
})

context('Request with get industry customers', () => {
const industry = ['Logistics', 'Retail', 'HR', 'Logistics']
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E a indústria Technology?

it.each(industry)('Filter to customers with industry', (industry) => {
cy.request(`${Cypress.env('baseUrl')}?page=1&industry=${industry}`).then(({status, body}) => {
expect(status).to.eq(200)
expect(body.customers[0].industry).to.eq(industry)
})
})
})
context('Request with get size customers', () => {
const sizes = ['Small','Medium','Enterprise','Large Enterprise','Very Large Enterprise']
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const sizes = ['Small','Medium','Enterprise','Large Enterprise','Very Large Enterprise']
const sizes = ['Small', 'Medium', 'Enterprise', 'Large Enterprise', 'Very Large Enterprise']

Ao deixar um espaço em branco entre cada item do array, a legibilidade melhora.

it.each(sizes)('Filter to customer with size', (sizes) => {
cy.request(`${Cypress.env('baseUrl')}?page=1&size=${sizes}&industry=All`).then(({status, body}) => {
expect(status).to.eq(200)
expect(body.customers[0].size).to.eq(sizes)
})
})
})

context('Bad Requests', () => {
it('Check message for invalid page', () => {
cy.request({url: `${Cypress.env('baseUrl')}?page=0`, failOnStatusCode: false}).then(({status, body}) => {
expect(status).to.eq(400)
expect(body.error).to.contain('Invalid page or limit. Both must be positive numbers.')
})
})

it('Check message for invalid limit', () => {
cy.request({url: `${Cypress.env('baseUrl')}?page=1&limit=0`, failOnStatusCode: false}).then(({status, body}) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se o teste é de invalid limit, pra que passar page?

expect(status).to.eq(400)
expect(body.error).to.contain('Invalid page or limit. Both must be positive numbers.')
})
})

it('Check message for invalid page with insert string', () => {
cy.request({url: `${Cypress.env('baseUrl')}?page=teste&limit=0`, failOnStatusCode: false}).then(({status, body}) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se o teste de de invalid page, pra que passar limit?

expect(status).to.eq(400)
expect(body.error).to.contain('Invalid page or limit. Both must be positive numbers.')
})
})

it('Check message for invalid limit with insert boolean', () => {
cy.request({url: `${Cypress.env('baseUrl')}?page=teste&limit=true`, failOnStatusCode: false}).then(({status, body}) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se o teste é de invalid limit, pra que passar page?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Além disso, que tal quebrar a linha para evitar scroll horizontal?

expect(status).to.eq(400)
expect(body.error).to.contain('Invalid page or limit. Both must be positive numbers.')
})
})

it('Check message for invalid size', () => {
const message = 'Unsupported size value. Supported values are All, Small, Medium, Enterprise, Large Enterprise, and Very Large Enterprise.'
cy.request({ url: `${Cypress.env('baseUrl')}?page=1&limit=10&size=0`, failOnStatusCode: false}).then(({status, body}) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se o teste é de invalid size, pra que passar page e limit?

expect(status).to.eq(400)
expect(body.error).to.contain(message)
})
})

it('Check message for invalid industry', () => {
const message = 'Unsupported industry value. Supported values are All, Logistics, Retail, Technology, HR, and Finance.'
cy.request({ url: `${Cypress.env('baseUrl')}?page=1&limit=10&size=All&industry=0`, failOnStatusCode: false}).then(({status, body}) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se o teste é de invalid industry, pra que passar page, limit e size?

expect(status).to.eq(400)
expect(body.error).to.contain(message)
})
})
})
})

50 changes: 50 additions & 0 deletions cypress/e2e/uitest/customers_gui.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/// <reference types="cypress" />

describe('Tests customers gui', {tags: 'gui', env:{baseUrl: 'http://localhost:3000/'}},() => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conforme já comentado, a baseUrl deveria ter sido definida no arquivo de configurações (cypress.config.js).

context('Home customers', () => {
beforeEach(() => {
cy.setCookieSession('accepted')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vale mesmo a pena abstrair isso para um comando customizado?

Além disso, o comando não diz o que ele realmente faz, pois além de definir o cookie, ele visita a página da aplicação em teste, mas isso não é claro ao ler o nome do comando.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A propósito, por qual motivo o comando visita a página, se logo abaixo há um cy.visit()?

cy.visit(Cypress.env('baseUrl'))
})

const industries = ['Logistic', 'Retail', 'Technology', 'HR', 'Finance']
it.each(industries)('Filter with industries', (industries) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Veja abaixo um teste meu para filtragem por indústria.

it('filters by Logistics industry', () => {
  cy.intercept(
    'GET',
    `${CUSTOMERS_API_URL}?page=1&limit=10&size=All&industry=Logistics`,
    { fixture: 'logisticsCustomers' }
  ).as('getLogisticsCustomers')

  cy.get('[data-testid="industry-filter"]').select('Logistics')

  cy.wait('@getLogisticsCustomers')

  cy.get('tbody tr').should('have.length', 1)
})

Você percebe como este teste é mais simples e direto ao ponto?

Além disso, perceba que só estou testando que o frontend está fazendo a chamada corretao ao backend.

Pra verificar que o backend filtra corretamente, testo no nível da API.

cy.intercept('GET', '**&industry=*').as('wait')
cy.get('[data-testid="industry-filter"]').select(industries)
cy.wait('@wait')

cy.get('[data-testid="table"] > table > tbody > tr').then((elements) => {
cy.wrap(elements).each(() => {
cy.contains('tr', industries)
})
})
})

const sizes = ['Small', 'Medium', 'Enterprise', 'Large Enterprise', 'Very Large Enterprise']
it.each(sizes)('Filter with sizes', (sizes) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mesmo que o comentário acima.

cy.intercept('GET', '**&size=*').as('wait')
cy.get('[data-testid="size-filter"]').select(sizes)
cy.wait('@wait')

cy.get('[data-testid="table"] > table > tbody > tr').then((elements) => {
cy.wrap(elements).each(() => {
cy.contains('tr', sizes)
})
})
})

it('Check maintain filter when returning from view details customers ', () => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Que tal a seguinte descrição?

Suggested change
it('Check maintain filter when returning from view details customers ', () => {
it('keeps the filters when coming back from the customer details view', () => {

cy.get('[data-testid="size-filter"]').as('select')
cy.get('@select').select('Small')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Em vez de fazer isso, a linha acima podia ser:

 cy.get('[data-testid="size-filter"]')
  .as('select')
  .select('Small')

cy.get('[data-testid="table"] > table > tbody > tr').as('list')
cy.get('@list').first().last().find('button').should('be.visible').click()
cy.contains('h2', 'Customer Details').should('be.visible')

cy.contains('button', 'Back').should('be.visible').click()

cy.get('@select').should('be.visible').invoke('val').should('eq', 'Small')
})
})
})

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Uma linha em branco ao final do arquivo já é o suficiente.


32 changes: 32 additions & 0 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// ***********************************************
// This example commands.js shows you how to
// create various custom commands and overwrite
// existing commands.
//
// For more comprehensive examples of custom
// commands please read more here:
// https://on.cypress.io/custom-commands
// ***********************************************
//
//
// -- This is a parent command --
// Cypress.Commands.add('login', (email, password) => { ... })
//
//
// -- This is a child command --
// Cypress.Commands.add('drag', { prevSubject: 'element'}, (subject, options) => { ... })
//
//
// -- This is a dual command --
// Cypress.Commands.add('dismiss', { prevSubject: 'optional'}, (subject, options) => { ... })
//
//
// -- This will overwrite an existing command --
// Cypress.Commands.overwrite('visit', (originalFn, url, options) => { ... })

Cypress.Commands.add('setCookieSession', (value) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Já falei anteriormente sobre a não-necessidade deste comando, visto que sua implementação é tão simples.

A propósito, o cy.visit() não faz sentido aqui, visto que logo após o uso deste comando você visita a página de novo, e também, pois o nome não descreve tal visita.

Por fim, você percebeu que abstraiu isso para um comando e tal comando só é usado uma vez?

Isso é o que chamo de uma abstração prematura.

cy.session(value ,() => {
cy.visit(Cypress.env('baseUrl'))
cy.setCookie('cookieConsent', value)
})
})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
})
})

Recomenda-se deixar uma linha em branco ao final de cada arquivo, para evitar que ferramentas como o GitHub exibam o ícone de No newline at end of file.

Isso é uma boa prática e convenção a ser seguida.

21 changes: 21 additions & 0 deletions cypress/support/e2e.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tem algum motivo pelo qual optaste por criar esse arquivo, ao invés de utilizar o cypress/support/commands.js?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Este arquivo faz parte da estrutura padrão do Cypress @ldasilvaNZ11.

Ele é o arquivo principal dos comandos customizados, onde plugins externos são importados e também comandos customizados (cypress/support/commands.j).

Ou seja, tudo certo!

Eu só removeria esse monte de comentários, pra dar uma limpada no arquivo.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Algum motivo por não ter removido os comentários?

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// ***********************************************************
// This example support/e2e.js is processed and
// loaded automatically before your test files.
//
// This is a great place to put global configuration and
// behavior that modifies Cypress.
//
// You can change the location of this file or turn off
// automatically serving support files with the
// 'supportFile' configuration option.
//
// You can read more here:
// https://on.cypress.io/configuration
// ***********************************************************

// Import commands.js using ES2015 syntax:
import 'cypress-each'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se tivesse implementado solução mais simples para os testes, tal lib não seria necessária.

import './commands'

// Alternatively you can use CommonJS syntax:
// require('./commands')
Loading