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

Add first cypress tests #2

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

pedrohyvo
Copy link

@pedrohyvo pedrohyvo commented Oct 8, 2024

Descrição

  • Qual o propósito desta PR?
    O propósito do PR é implementar testes e2e para a aplicação EngageSphere no nível Avançado.

  • O que foi alterado?
    Foi adicionar o cypress juntamente com alguns testes e2e ao repositório.

  • Qual problema está sendo resolvido? (se aplicável)

Issue Relacionada

  • Closes #[Número da Issue]

Tipo de Mudança

  • Bugfix (correção de um problema)
  • Nova feature (adicionando uma funcionalidade)
  • Refatoração (alteração sem impacto na funcionalidade)
  • Documentação (adição ou alteração na documentação)
  • Testes (adição/alteração de testes)

Como isso foi testado?

  • Testes unitários
  • Testes de integração
  • Testado manualmente (informe como)
  • Testes automatizados (CI/CD)

Checklist

  • Eu realizei um auto-review do meu código
  • Eu comentei meu código, principalmente em áreas difíceis de entender
  • Eu adicionei testes que cobrem as mudanças (se necessário)
  • As mudanças estão seguindo os padrões do projeto
  • A documentação foi atualizada (se necessário)

Capturas de Tela (se aplicável)

Outras Observações

@wlsf82 wlsf82 requested a review from jonathanrsbr October 8, 2024 17:24
Copy link
Collaborator

@jonathanrsbr jonathanrsbr left a comment

Choose a reason for hiding this comment

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

Parabéns pelo trabalho, Pedro!

Foste um dos primeiros a entregar a PR e conseguiste implementar vários testes já nessa rodada. Foi bacana analisar seu código.

Gostei da forma como lidaste com os cookies e como usou a BaseURL. Muito bom!

Sugiro sempre adicionar uma descrição na PR. Isso ajuda o revisor a entender o que foi acrescentado. Tenho um padrão bem bacana, vou te mandar via comentário adicional.

Se possível, aguarda a próxima aula para fazer as correções, aposto que já vais ter outros insights!

Sucesso nos próximos testes!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cypress.config.js Outdated Show resolved Hide resolved
frontend/cypress.config.js Show resolved Hide resolved
frontend/cypress.config.js Show resolved Hide resolved
frontend/cypress/fixtures/example.json Show resolved Hide resolved
@@ -0,0 +1,25 @@
// ***********************************************
Copy link
Collaborator

Choose a reason for hiding this comment

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

Recomendo remover a pasta support, pois não foi criado nenhum custom-command.

Além de apontar para o não uso da pasta fixtures, deves acrescentar um argumento que não estava no arquivo de configuração na raiz do projeto, no caso, supportFile: false.

Segue exemplo:

// cypress.config.js

const { defineConfig } = require('cypress')

module.exports = defineConfig({
  e2e: {
    baseUrl: '...',
    supportFile: false
  },
  fixturesFolder: false,
})

Copy link
Owner

Choose a reason for hiding this comment

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

Mesma coisa aqui.

Copy link
Owner

Choose a reason for hiding this comment

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

Mais uma conversa resolvida sem a implementação da sugestão e sem uma justificativa para a não-implementação da mesma.

frontend/cypress/support/e2e.js Show resolved Hide resolved
@jonathanrsbr
Copy link
Collaborator

jonathanrsbr commented Oct 10, 2024

Como prometido, segue um padrão para descrição de PR, evidentemente que usas somente o que for necessário para a ocasião! haahaha

Descrição

  • Qual o propósito desta PR?
  • O que foi alterado?
  • Qual problema está sendo resolvido? (se aplicável)

Issue Relacionada

Closes #[Número da Issue]

Tipo de Mudança

  • Bugfix (correção de um problema)
  • Nova feature (adicionando uma funcionalidade)
  • Refatoração (alteração sem impacto na funcionalidade)
  • Documentação (adição ou alteração na documentação)
  • Testes (adição/alteração de testes)

Como isso foi testado?

  • Testes unitários
  • Testes de integração
  • Testado manualmente (informe como)
  • Testes automatizados (CI/CD)

Checklist

  • Eu realizei um auto-review do meu código
  • Eu comentei meu código, principalmente em áreas difíceis de entender
  • Eu adicionei testes que cobrem as mudanças (se necessário)
  • As mudanças estão seguindo os padrões do projeto
  • A documentação foi atualizada (se necessário)

Capturas de Tela (se aplicável)

Outras Observações

Copy link
Owner

@wlsf82 wlsf82 left a comment

Choose a reason for hiding this comment

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

@pedrohyvo, parabéns pela resolução deste primeiro round dos exercícios.

Deixei alguns comentários também, os quais creio que lhe ajudarão a melhorar ainda mais o design dos seus testes.

Aguarde até o próximo encontro antes de implementar as mudanças.

cypress.config.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/top.cy.js Outdated Show resolved Hide resolved
Copy link
Owner

@wlsf82 wlsf82 left a comment

Choose a reason for hiding this comment

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

Mais uma vez parabéns pelos ajustes @pedrohyvo! 👏🏻

Deixei mais alguns comentários os quais creio que lhe ajudarão a melhorar o design dos testes.

Lembre-se de só enviar novas mudanças para revisão após o próximo sábado (19/10/2024).


To run cypress tests, you will need to run the following command:
```
Interactive: npm run cy:open
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
Interactive: npm run cy:open
Interactive: `npm run cy:open`

Ao colocar o comando entre aspas, o GitHub o renderiza como código, visto a sintaxe de Markdown.

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 resolva conversas sem implementar a sugestão, ou sem uma justificativa para a não-implementação.

To run cypress tests, you will need to run the following command:
```
Interactive: npm run cy:open
Headless: npm test or npm t
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
Headless: npm test or npm t
Headless: `npm test or npm t`

Mesma coisa aqui.

cypress/e2e/home/home.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/home.cy.js Outdated Show resolved Hide resolved
cypress/e2e/home/home.cy.js Outdated Show resolved Hide resolved
frontend/cypress/fixtures/example.json Show resolved Hide resolved
@@ -0,0 +1,25 @@
// ***********************************************
Copy link
Owner

Choose a reason for hiding this comment

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

Mesma coisa aqui.

@@ -0,0 +1,20 @@
// ***********************************************************
Copy link
Owner

Choose a reason for hiding this comment

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

E aqui.

Copy link
Owner

Choose a reason for hiding this comment

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

Mais uma conversa resolvida sem a implementação da sugestão e sem uma justificativa para a não-implementação da mesma.

jsconfig.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@pedrohyvo pedrohyvo requested a review from wlsf82 October 28, 2024 12:52
@wlsf82 wlsf82 removed the request for review from jonathanrsbr October 29, 2024 19:05
Copy link
Owner

@wlsf82 wlsf82 left a comment

Choose a reason for hiding this comment

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

Parabéns por mais uma rodada de implementação dos exercícios @pedrohyvo. 👏🏻

Deixei novos comentários os quais espero que lhe ajudem a melhorar suas habilidades de design de testes.

Espero que tenha gostado da experiência de code review da Test Design Masterclass da Talking About Testing.

env: {
API_URL: '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.

Suggested change
})
})

Cuidado com a indentação!


To run cypress tests, you will need to run the following command:
```
Interactive: npm run cy:open
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 resolva conversas sem implementar a sugestão, ou sem uma justificativa para a não-implementação.

})

context('Customer Details', () => {
it.only('It shows and hides the customer address', () => {
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
it.only('It shows and hides the customer address', () => {
it('It shows and hides the customer address', () => {

Cuidado para não fazer o commit de testes com .only.

context('Customer Details', () => {
it.only('It shows and hides the customer address', () => {
cy.contains('button', 'View').click()
cy.contains('h2', 'Customer Details').should('be.visible')
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
cy.contains('h2', 'Customer Details').should('be.visible')
cy.contains('h2', 'Customer Details').should('be.visible')

it.only('It shows and hides the customer address', () => {
cy.contains('button', 'View').click()
cy.contains('h2', 'Customer Details').should('be.visible')
cy.get('button[class*="CustomerDetails_showAddressBtn"]').click()
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
cy.get('button[class*="CustomerDetails_showAddressBtn"]').click()
cy.get('button[class*="CustomerDetails_showAddressBtn"]').click()

"totalPages": 1,
"totalCustomers": 1
}
}
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
}
}


module.exports = defineConfig({
e2e: {
setupNodeEvents(on, config) {
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 resolva conversas sem implementar a mudança sugerida.

E se tiver um motivo para não implementar a sugestão, deixe aqui um contra-argumento sem resolver a conversa, para que o/a revisor(a) possa entender sua justificativa.

@@ -0,0 +1,5 @@
{
"name": "Using fixtures to represent data",
Copy link
Owner

Choose a reason for hiding this comment

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

Faltou deletar essa fixture visto que ela não está em uso.

@@ -0,0 +1,25 @@
// ***********************************************
Copy link
Owner

Choose a reason for hiding this comment

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

Mais uma conversa resolvida sem a implementação da sugestão e sem uma justificativa para a não-implementação da mesma.

@@ -0,0 +1,20 @@
// ***********************************************************
Copy link
Owner

Choose a reason for hiding this comment

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

Mais uma conversa resolvida sem a implementação da sugestão e sem uma justificativa para a não-implementação da mesma.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants