-
Notifications
You must be signed in to change notification settings - Fork 398
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
Tests e2e utilizando playwright com relatório #1716
base: main
Are you sure you want to change the base?
Tests e2e utilizando playwright com relatório #1716
Conversation
@nobregao is attempting to deploy a commit to the TabNews Team on Vercel. A member of the Team first needs to authorize it. |
os testes dessa PR foram desenvolvidos focados em dois perfis de usuário: anônimo e padrão. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nobregao muito obrigado pela implementação!
O PR me parece estar no caminho certo. Revisei alguns pontos (não vi o PR inteiro), e acredito que a implementação ficará mais simples. Também sugeri testes novos.
Escolha de seletores
A melhor opção é realizar a seleção com base no texto e atributos de acessibilidade. Isso garante um comportamento mais similar ao do usuário, além de garantir que o elemento é acessível da forma esperada, de não precisar alterar os componentes para adicionar um data-test-id
e nem precisar inspecionar o código para criar um teste.
Pode ler mais sobre isso em Making your UI tests resilient to change, um artigo escrito pelo criador da biblioteca @testing-library/react
. O Playwright também tem recomendações sobre isso.
Ou seja, para encontrar um link, podemos assegurar tanto que ele é âncora quanto o texto que ele exibe:
await page.getByRole('a', { name: 'Relevantes' });
Para preencher um formulário, podemos garantir isso com:
await page.getByLabel('Senha');
E assim por diante.
.github/workflows/ci.yml
Outdated
unit-tests: | ||
name: Unit Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Estes não são apenas testes de unidade, são de integração também. Não sei como poderíamos nomear este grupo que é "todos os testes com exceção dos e2e".
Faz sentido colocar os testes e2e no mesmo job?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Como precisamos que testes sejam executados em todo e qualquer PR aberto, acredito que faz sentido colocar em um job só. Com essa ideia podemos dividir em etapas de execução, algo pensei abaixo:
tests:
name: Tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Start containers
run: npm run services:up
- uses: actions/cache@v2
with:
path: ${{ github.workspace }}/.next/cache
key: ${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-${{ hashFiles('**.[jt]s', '**.[jt]sx') }}
restore-keys: |
${{ runner.os }}-nextjs-${{ hashFiles('**/package-lock.json') }}-
- uses: actions/setup-node@v2
with:
node-version: '18'
cache: 'npm'
- name: Install all dependencies
run: npm ci
- run: npx playwright install --with-deps
- name: Run unit and integrations tests
run: npm run dev & npx vitest run
- name: Run e2e tests
run: npm run dev & npx playwright test
- uses: actions/upload-artifact@v4
if: ${{ !cancelled() }}
with:
name: playwright-report
path: playwright-report/
retention-days: 30
- name: Stop containers
if: always()
run: npm run services:down
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eu não tenho certeza se faz sentido juntar no mesmo teste pelo tempo que demoraria para executar. Pensei nisso pelo tempo de demora do setup, mas na verdade demora menos de 40 segundos. Pode deixar separado como está, a não ser que alguém traga uma informação nova que pareça fazer sentido deixar todos juntos.
Precisa apenas mudar o nome unit tests
mesmo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talvez os scripts test
e test:watch
possam continuar realizando todos os testes, incluindo agora os com Playwright, mas também deverão existir scripts separados para cada caso.
A separação não precisa ser necessariamente em unit
, integration
e e2e
. Acredito que faça sentido separar por ferramentas de teste, e se dependem ou não de subir o servidor Next.js, banco de dados, etc.
Só um exemplo sem pensar muito:
- test - realiza todos os testes (
npm run test:unit && npm run test:services && npm run test:browsers
). - test:unit - testes de unidade (filtro
/unit
). - test:services - testes que dependam de subir todos os serviços (a maioria dos testes atuais, mas adiciona os filtros
/integration
e/e2e
). - test:browsers - testes que simulam a interação pelo browser (nova possibilidade com o Playwright, e o filtro está em
playwright.config.js
).
Com scripts separados, temos liberdade para decidir entre executar tudo no mesmo job ou em jobs separados, de acordo com o custo benefício. Rodando separadamente, ganhamos velocidade no CI, mas isso implica em maior uso da cota do GitHub Actions, então é algo que tem que ser fácil de mudar conforme a necessidade.
E já que toquei no assunto da cota, esse é um dos motivos para cuidarmos em só colocar nos testes e2e o que for realmente necessário, pois é tempo valioso dos desenvolvedores aguardando a execução dos testes, mas também pode implicar em custos no GitHub.
@@ -82,13 +82,15 @@ Por padrão, ao rodar o comando `npm run dev` será injetado dois usuários ativ | |||
|
|||
## Rodar os testes | |||
|
|||
Há várias formas de rodar os testes dependendo do que você deseja fazer, mas o primeiro passo antes de fazer qualquer alteração no projeto é rodar os testes de forma geral para se certificar que tudo está passando como esperado. O comando abaixo irá rodar todos os serviços necessários, rodar os testes e em seguida derrubar todos os serviços. | |||
### Testes unitários |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Como dito acima, estes não são apenas testes de unidade, são de integração também. Acredito que não precise criar esse tópico, mas o tópico para testes e2e pode continuar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
entendi. sendo assim, faz sentido o subtítulo dessa seção ser algo como abaixo?
Testes unitários e Testes de integração
se sim, irei remover também a menção única a testes unitários dessa seção e deixar mais abrangente.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho que fica melhor 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Se os scripts atuais test
, test:watch
e test:watch:services
forem executar também os testes com Playwright, então nem precisará de um tópico separado, sendo necessário apenas algum ajuste para citar isso.
Se acharem melhor manter um tópico próprio, "Testes de Integração" pode ser um sub-tópico de "Rodar os testes". Nesse caso, falta arrumar algo na documentação, ou nos scripts, para realmente conseguir executar os testes apenas seguindo o documentado aqui. Falta a instalação das dependências do Playwright, mas deve ser melhor colocar isso no script do que na documentação.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Falta a instalação das dependências do Playwright, mas deve ser melhor colocar isso no script do que na documentação.
Eu tenho usado o Fedora para programar o TabNews, e o Fedora não é suportado pelo Playwright. Apesar disso, consegui rodar os testes headless usando Docker. Acham que vale a menção disso na documentação? Usei esta referência com uma pequena adaptação para compartilhar o repositório do projeto entre o host e o container:
docker run -it --rm \
-v /home/rafael/Documents/Repositorios/tabnews.com.br:/tabnews.com.br --net=host \
--ipc=host mcr.microsoft.com/playwright:v1.44.1-jammy /bin/bash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
o Fedora não é suportado pelo Playwright.
Interessante, então podemos deixar um docker-compose.yml
pronto pra esses casos. Acho mais simples de documentar que é possível usar outro script em sistemas não compatíveis com o Playwright. Por exemplo:
"script:alternativo": "docker compose -f infra/docker-compose.test.yml up"
services:
playwright:
container_name: playwright
image: mcr.microsoft.com/playwright:v1.44.1-jammy
volumes:
- ..:/playwright
network_mode: "host"
ipc: "host"
working_dir: /playwright
command: >
/bin/bash -c '
npx playwright install --with-deps
&& npm run test:e2e
'
Outra coisa que falta na documentação, ou no script test:e2e
, é que é preciso subir os serviços antes de rodar o teste e2e. Melhor adicionar ao script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Olha que interessante!
No Codespaces o Playwright parece funcionar muito mais estável dentro do contêiner docker do que rodando o teste direto pelo terminal. No docker não vi falhar nenhuma vez. Quando executando direto no terminal, muitas vezes acaba passando dos 30s de timeout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nobregao você consegue criar o exemplo que o @aprendendofelipe sugeriu e executar os testes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ainda não revisei todos os testes criados, mas parece que tem bastante coisa para ajustar nos que eu já vi. Não só por esse motivo, mas estou achando melhor concluir o debate sobe a configuração que permita os testes com Playwright, e depois implementar os testes em si, em outro PR.
Sobre a configuração, para ajudar em algumas coisas que comentei no código, estou pensando na seguinte estrutura de pastas para os testes. Vejam se isto faz sentido:
tests/
├── e2e/ # Nova pasta para os testes end-to-end
| ├── api/ # Trazer de `/integration/api/v1/_use-cases...`
| | ├── registration # `.../registration-flow.test.ts`
| | └── ...
| |
│ └── browser/ # Testes E2E que simulam o navegador
│ ├── login
│ ├── publish
│ ├── registration
│ └── ...
│
├── integration/
│ ├── api/ # Mesma estrutura atual, com exceção de `v1/_use-cases/`
│ │
│ ├── browser/ # Testes de integração que dependem do navegador, como
│ │ └── ... # de UI mais complexos que não forem possíveis com
│ │ # React Testing Library, mas que não são testes E2E
│ │
│ └── ...
│
├── unit/ # Mesma estrutura atual
│ ├── infra/
│ ├── interface/
│ └── ...
│
└── helpers/
├── browser/ # Auxiliares para testes E2E
│ ├── page-objects/
│ └── ...
│
├── constants
├── orchestrator
│
└── ... # Outros auxiliares comuns
Coloquei a pasta page-objects
no plural. Faz sentido? Outras opções menos interessantes são pages
ou screens
. E faz sentido colocar na pasta helpers
? Não sei se seria utilizado apenas nos e2e
ou se em qualquer teste com Playwright. Aliás, vale a pena a gente já adotar o modelo page objects? Não seria overengineering?
.github/workflows/ci.yml
Outdated
unit-tests: | ||
name: Unit Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talvez os scripts test
e test:watch
possam continuar realizando todos os testes, incluindo agora os com Playwright, mas também deverão existir scripts separados para cada caso.
A separação não precisa ser necessariamente em unit
, integration
e e2e
. Acredito que faça sentido separar por ferramentas de teste, e se dependem ou não de subir o servidor Next.js, banco de dados, etc.
Só um exemplo sem pensar muito:
- test - realiza todos os testes (
npm run test:unit && npm run test:services && npm run test:browsers
). - test:unit - testes de unidade (filtro
/unit
). - test:services - testes que dependam de subir todos os serviços (a maioria dos testes atuais, mas adiciona os filtros
/integration
e/e2e
). - test:browsers - testes que simulam a interação pelo browser (nova possibilidade com o Playwright, e o filtro está em
playwright.config.js
).
Com scripts separados, temos liberdade para decidir entre executar tudo no mesmo job ou em jobs separados, de acordo com o custo benefício. Rodando separadamente, ganhamos velocidade no CI, mas isso implica em maior uso da cota do GitHub Actions, então é algo que tem que ser fácil de mudar conforme a necessidade.
E já que toquei no assunto da cota, esse é um dos motivos para cuidarmos em só colocar nos testes e2e o que for realmente necessário, pois é tempo valioso dos desenvolvedores aguardando a execução dos testes, mas também pode implicar em custos no GitHub.
@@ -82,13 +82,15 @@ Por padrão, ao rodar o comando `npm run dev` será injetado dois usuários ativ | |||
|
|||
## Rodar os testes | |||
|
|||
Há várias formas de rodar os testes dependendo do que você deseja fazer, mas o primeiro passo antes de fazer qualquer alteração no projeto é rodar os testes de forma geral para se certificar que tudo está passando como esperado. O comando abaixo irá rodar todos os serviços necessários, rodar os testes e em seguida derrubar todos os serviços. | |||
### Testes unitários |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Se os scripts atuais test
, test:watch
e test:watch:services
forem executar também os testes com Playwright, então nem precisará de um tópico separado, sendo necessário apenas algum ajuste para citar isso.
Se acharem melhor manter um tópico próprio, "Testes de Integração" pode ser um sub-tópico de "Rodar os testes". Nesse caso, falta arrumar algo na documentação, ou nos scripts, para realmente conseguir executar os testes apenas seguindo o documentado aqui. Falta a instalação das dependências do Playwright, mas deve ser melhor colocar isso no script do que na documentação.
- name: Run e2e tests | ||
run: npm run dev & npx playwright test | ||
|
||
- uses: actions/upload-artifact@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Por que fazer esse upload?
9fe6b98
to
ad90251
Compare
sendo assim topam cancelar esse PR? dado isso, abro outro com a configuração e, depois, outro com os cenários. ✌️ |
acredito não faça sentido estar no plural e renomeá-la, dado que |
a princípio para esse PR penso que deveria estar com e2e dado que é somente utilizado ali. |
acredito que valha, inclusive, por usarmos o next.js com |
Pode trabalhar a configuração neste PR mesmo, para não fragmentarmos o histórico da discussão. Sugiro deixar ao menos um teste simples para garantirmos que está funcionando corretamente. Um exemplo onde isso foi feito é no PR #1668, onde fiz uma refatoração e criei alguns testes bem simples para um componente, apenas para garantir que a configuração da biblioteca |
vou reorganizar os commits para simplificar a análise final ✌️ |
Fala @nobregao, Você solicitou uma nova revisão, mas ainda há pontos anteriores que precisam ser resolvidos. Já revisou todos os comentários que fizemos? É normal discordar dos revisores, mas é importante dar um retorno sobre cada ponto. Para facilitar a organização, tanto para você quanto para os revisores, marque os comentários como resolvidos apenas quando tiver certeza de que foram. Se não houver consenso, responda ao questionamento e deixe a marcação para os revisores. Se for algo que você resolverá mais tarde, mantenha em aberto para não esquecer de tratar até que o código esteja de acordo com o resultado da revisão. Precisa de ajuda com algo específico? |
dada a priorização do PR deixo a configuração para execução dos testes e apenas o teste da página de login. os últimos commits foram para essa organização. removendo arquivos e trocando o atributo que acham @aprendendofelipe @Rafatcb? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dada a priorização do PR deixo a configuração para execução dos testes e apenas o teste da página de login.
Me parece o ideal 👍
@@ -130,6 +130,10 @@ async function createUser(userObject) { | |||
}); | |||
} | |||
|
|||
async function findUserByEmail(email) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Não está mais usando isso, certo? Pode remover as alterações aqui.
@@ -88,7 +88,11 @@ function LoginForm() { | |||
<> | |||
<form style={{ width: '100%' }} onSubmit={handleSubmit}> | |||
<Box sx={{ display: 'flex', flexDirection: 'column', gap: 3 }}> | |||
{globalErrorMessage && <Flash variant="danger">{globalErrorMessage}</Flash>} | |||
{globalErrorMessage && ( | |||
<Flash variant="danger" aria-label="Mensagem de erro"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provavelmente não precisa de aria-label
aqui e nem nos outros lugares. O ideal é não precisar mexer no HTML (JSX), e usar algum seletor como mencionei em #1716 (review), especificando o texto, e então ao invés de comparar "se tem o texto esperado", seria uma comparação do tipo "se foi encontrado".
Não clonei a branch agora para testar, mas havia testado isso quando criei o outro comentário. Se precisar de ajuda para definir o seletor sem precisar alterar o HTML, pode falar.
@@ -82,13 +82,15 @@ Por padrão, ao rodar o comando `npm run dev` será injetado dois usuários ativ | |||
|
|||
## Rodar os testes | |||
|
|||
Há várias formas de rodar os testes dependendo do que você deseja fazer, mas o primeiro passo antes de fazer qualquer alteração no projeto é rodar os testes de forma geral para se certificar que tudo está passando como esperado. O comando abaixo irá rodar todos os serviços necessários, rodar os testes e em seguida derrubar todos os serviços. | |||
### Testes unitários |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nobregao você consegue criar o exemplo que o @aprendendofelipe sugeriu e executar os testes?
@@ -7,14 +7,32 @@ concurrency: | |||
cancel-in-progress: true | |||
|
|||
jobs: | |||
tests: | |||
name: Tests | |||
lint-styles: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho que esse arquivo ficou mais confuso de revisar porque mudou a ordem dos jobs e voltou a usar as actions na v2
, sendo que atualizamos para a v4
.
@nobregao você pode reorganizar aqui para simplificar o diff? E também voltar a usar as versões atuais das actions.
Edit: vi agora que está conflitando com o main
, então o rebase
deve ajudar a corrigir isso.
Mudanças realizadas
Implementado testes de integração e2e com playwright na versão
1.43.1
utilizando o padrão page object seguindo a discussão na issue #1667.O objetivo desse PR é a configuração inicial do playwright e a implementação de testes simples.
Scripts de execução de testes adicionados (mais detalhes no
README.md
):test:e2e
executa com browser em segundo planotest:e2e:headed
executa abrindo browsertest:e2e:ui
executa com uma interface que permite acompanhar execução numa timelineÚltima execução de pipeline pode ser verificada aqui com relatório no fim da página em Artifacts
Tipo de mudança
Checklist: