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

Testes e2e utilizando playwright #1687

Closed
wants to merge 60 commits into from

Conversation

nobregao
Copy link

@nobregao nobregao commented May 4, 2024

Mudanças realizadas

Implementado testes de integração e2e com playwright 1.43.1 utilizando o padrão page object.

Scripts adicionados:

  • test:e2e executa os testes automatizados
  • test:e2e:ui: utiliza ui mode em que é possível executar e debugar scripts e visualizar html por etapa de execução.

Objetivo

Seguir com discussão entre escolha de tecnologias para testes e2e da issue #1667

Foram incrementando poucos cenários para analisar: tempo de execução, sintaxe e configuração de dependências

A ideia é a partir da escolha entre as tecnologias incrementar cobertura da suíte de testes com cenários importantes abrangendo os 3 perfis de usuários (anônimo, padrão e privilegiado)

Atividade para esse PR

  • Verificar execução dos testes com github actions

Testes

Cenários implementados por perfil de usuário

Anônimo (Usuário não logado)

  • Consegue acessar área de login
  • Consegue acessar área de registro
  • Consegue acessar área de conteúdo relevantes e recentes
  • Consegue realizar login como usuário padrão

Padrão (Usuário sem privilégios)

  • Consegue publicar conteúdo através do cabeçalho
  • Consegue publicar conteúdo através do menu de usuário

Tipo de mudança

  • Nova funcionalidade

Checklist:

  • As modificações não geram novos logs de erro ou aviso (warning).
  • Eu adicionei testes que provam que a correção ou novo recurso funciona conforme esperado.
  • Tanto os novos testes quanto os antigos estão passando localmente.

Copy link

vercel bot commented May 4, 2024

@nobregao is attempting to deploy a commit to the TabNews Team on Vercel.

A member of the Team first needs to authorize it.

@nobregao nobregao marked this pull request as draft May 4, 2024 17:47
Copy link
Collaborator

@aprendendofelipe aprendendofelipe left a comment

Choose a reason for hiding this comment

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

@nobregao, obrigado pelo PR! 💪💪💪

Scripts adicionados:

  • test:e2e executa os testes automatizados
  • test:e2e:ui: utiliza ui mode em que é possível executar e debugar scripts e visualizar html por etapa de execução.

Os scripts ainda não foram adicionados, então vejo que é um trabalho em andamento, certo? Vi que você mudou agora o PR para draft, então vamos manter assim enquanto a implementação não estiver funcional. 🤝

Olhei por cima a parte de configuração e já deixei alguns comentários no código. 👍

.env.test Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
playwright.config.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@Rafatcb Rafatcb left a comment

Choose a reason for hiding this comment

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

Muito obrigado pela contribuição, @nobregao!

Além dos comentários que deixei, acho importante adicionar no README como executar esse tipo de teste, o modo watch, e como testar arquivos específicos. Algo como a documentação que temos hoje para os outros tipos de teste (veja aqui). Sugiro deixar isso para o final, comentei apenas para não esquecermos.

Um exemplo de link que acredito ser importante de constar nas orientações é o [TIP] Run Playwright Tests on unsupported Linux distributions.

Outro ponto, não acham melhor seguirmos a lógica dos testes de integração, onde um arquivo representa um tipo de ação, ao invés de um arquivo representar um tipo de usuário? Os testes user-default podem acabar ficando bem extensos, já que o usuário padrão pode fazer várias coisas diferentes, além de não estar claro que tipo de comportamento foi testado pelo nome do arquivo.

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
tests/e2e/user-anonymous.spec.js Outdated Show resolved Hide resolved
tests/e2e/page-object/home-page.js Outdated Show resolved Hide resolved
playwright.config.js Show resolved Hide resolved
Comment on lines 25 to 26
let title = await homePage.getTitle();
await expect(title).toBe(titleHome);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Se isso for apenas para aguardar a página carregar, como perguntei em outro comentário, então realizando as alterações propostas a verificação do title pode ficar em um único teste, e os outros testes usarão apenas o método para aguardar o carregamento.

E, se isso for feito, não precisamos mais da variável titleHome.

* @param {import('@playwright/test').Page} page
* @param {string} postTitle
*/
constructor(page, postTitle) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

O construtor precisa receber o postTitle?

tests/e2e/page-object/user-post-page.js Outdated Show resolved Hide resolved
tests/e2e/user-anonymous.spec.js Outdated Show resolved Hide resolved
await recentPage.goRelevantTab();
});

test('should be login like user default', async ({ page }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alguns nomes de testes ficaram estranhos. Desse arquivo, acredito que só esse precise ser melhorado, mas do outro arquivo, é preciso rever todos. Isso não é urgente, pode deixar para o final porque assim já garante que todos os testes estarão adequados (tanto os atuais quanto os novos, caso você crie mais nesse PR).

Seguindo o padrão dos outros testes desse arquivo, aqui poderia ser: test('should be able to login' .

@Rafatcb Rafatcb added the testes Criação ou melhoria de testes label May 11, 2024
@aprendendofelipe
Copy link
Collaborator

@nobregao, vamos combinar de abrir um novo PR quando a solução estiver mais madura e pronta para ser revisada? 🤝

Enquanto isso, qualquer questão pode ser tratada pela issue #1667, e quem quiser acompanhar esse desenvolvimento mais de perto, commit a commit, pode dar watch no seu repositório: https://github.com/nobregao/tabnews.com.br

Não interpretem como desinteresse na solução, pois acreditamos que testes e2e são muito importantes. Mas vou fechar o PR atual pelos seguintes motivos:

  1. A quantidade de notificações que a Turma está recebendo a cada commits indica que você ainda está trabalhando bastante nessa solução. 💪 Só que essa quantidade de notificações pode ser interpretada como spam, fazer a Turma se desinscrever do PR ou, pior, do repositório.
  2. A grande quantidade de commits (já chegou a 60) também pode dificultar a revisão, então vou lhe pedir para reorganizar os commits antes de abrir o novo PR. 🤝
  3. Percebemos que muitos PRs que não são concluídos em poucas dias (ou semanas) acabam permanecendo indefinidamente sem conclusão, então estamos evitando deixar PRs abertos por muito tempo, a não ser que esteja havendo interação constante para revisão.

@nobregao nobregao deleted the tests-e2e-playwright branch June 4, 2024 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pendente Aguardando ação do autor do Pull Request testes Criação ou melhoria de testes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants