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

public-key-creation #42

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Fabricioperrone
Copy link

Este Pull Request implementa um fluxo automatizado utilizando o Cypress para acessar a página http://localhost/admin/deploy_keys/new. O objetivo é realizar operações CRUD (Create, Read, Update, Delete) utilizando comandos customizados.

Os passos do teste são estruturados da seguinte forma:

Ao acessar a página, o teste insere um título aleatório usando a biblioteca "Faker" do Cypress.

Em seguida, é criada uma chave (key) utilizando uma constante que previamente armazena o valor da chave. Após inserir os dados necessários, o teste clica no botão para confirmar a criação.

O próximo comando customizado verifica o número de chaves existentes. Se houver apenas uma chave, o teste a deleta. Se não houver nenhuma chave, o teste é considerado bem-sucedido.

Este fluxo garante a verificação do ciclo completo de criação, manipulação e exclusão de chaves, assegurando a funcionalidade esperada do sistema.

Cc.@wlsf82

@wlsf82
Copy link
Owner

wlsf82 commented Nov 29, 2023

O guia de escrita de commits não está sendo seguido.
Favor revisar e ajustar isto.

@@ -0,0 +1,22 @@
describe('Deploy Key Creation', () => {
Copy link
Owner

@wlsf82 wlsf82 Nov 29, 2023

Choose a reason for hiding this comment

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

O nome do arquivo deveria seguir a convenção camelCase, ou seja, em vez de deploykeyCreation.cy.js, devia ser deployKeyCreation.cy.js.

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, visto que o teste é do domínio admin, o mesmo deveria estar na pasta cypress/e2e/gui/admin/

describe('Deploy Key Creation', () => {
beforeEach(() => cy.sessionLogin())

it('creates an key ', () => {
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('creates an key ', () => {
it('CRUDS a deploy key successfully', () => {

Copy link
Owner

@wlsf82 wlsf82 Nov 29, 2023

Choose a reason for hiding this comment

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

Além disso, visto que isso é um teste de CRUD, faltou o update.

Ao testar o update, verificar mensagem de sucesso, além de verificar que o título foi atualizado.

beforeEach(() => cy.sessionLogin())

it('creates an key ', () => {
/**
Copy link
Owner

Choose a reason for hiding this comment

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

Remover comentários desnecessários.

@@ -281,3 +281,23 @@ Cypress.Commands.add('assertStatus', statusText => {
.should('contain', statusText)
cy.get('.qa-user-avatar').click()
})
// CRUD
Copy link
Owner

@wlsf82 wlsf82 Nov 29, 2023

Choose a reason for hiding this comment

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

Remover comentário e adicionar uma linha em branco para seguir a consistência.

@@ -281,3 +281,23 @@ Cypress.Commands.add('assertStatus', statusText => {
.should('contain', statusText)
cy.get('.qa-user-avatar').click()
})
// CRUD
Cypress.Commands.add('gui_deployKeyCreationName', (name = faker.string.uuid()) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Os comandos gui_deployKeyCreationName e gui_deployKeyCreation poderiam ser unificados em um só.
Ou melhor, que tal deixar a lógica direto no arquivo de teste?

.should('contain', '1')
cy.get('.btn-remove').click()
cy.get('.page-title')
.should('contain', '0')
Copy link
Owner

Choose a reason for hiding this comment

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

Verificações de resultados esperados não devem (de preferências) ser abstraídos para custom commands.
Quero ler o arquivo de teste e entender o que ele faz sem precisar ler nenhum outro arquivo.

@@ -0,0 +1,49 @@
-----BEGIN OPENSSH PRIVATE KEY-----
Copy link
Owner

@wlsf82 wlsf82 Nov 29, 2023

Choose a reason for hiding this comment

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

Deletar fixtures não utilizadas.
A propósito, private e public keys nunca deveriam ser versionadas.

Cypress.Commands.add('gui_deployKeyCreation',() => {
const publicKey = 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIF55ZxNcdW524Ob/y6kFkYu92DjTi2bqLkz37AFgFpZ+ root@example.com'
cy.get('#deploy_key_key').type(publicKey)
cy.get('.btn-success').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('.btn-success').click()
cy.get('input[type="submit"][value="Create"]').click()

Copy link
Owner

@wlsf82 wlsf82 Nov 29, 2023

Choose a reason for hiding this comment

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

Após a criação da key, gostaria de uma verificação de que a mesma foi criada. Algo como o seguinte:

cy.contains(‘.deploy-keys-list table tbody tr’, title).should(‘be.visible’)


})
Cypress.Commands.add('gui_deleteKey', () => {
cy.get('.page-title')
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 assim?

cy.contains(‘h3’, ‘Public deploy keys (1)’).should(‘be.visible’)

cy.get('.page-title')
.should('contain', '1')
cy.get('.btn-remove').click()
cy.get('.page-title')
Copy link
Owner

Choose a reason for hiding this comment

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

Aqui também

cy.contains(‘h3’, ‘Public deploy keys (0)’).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.

Além disso, seria interessante verificar que a tabela de keys não existe mais.
Algo assim:

Definir um cy.intercept().as('alias') pro POST que remove a deploy key antes do clique no botão que à remove, em conjunto com cy.wait(‘@alias’) depois do clique no botão. E então um cy.contains(‘.deploy-keys-list table tbody tr’, title).should(‘not exist’)

@@ -13,7 +14,9 @@ import './profile/setStatus.cy.js'
import './snippets/createSnippet.cy.js'
import './authentication/loginAsNonDefaultUser.cy'


Copy link
Owner

Choose a reason for hiding this comment

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

Por qual motivo você adicionou essa nova linha em branco? Favor removê-la.

@wlsf82
Copy link
Owner

wlsf82 commented Nov 29, 2023

Verificar erros de lint. Sugiro rodar um npm run lint:fix para correção automática de alguns dos erros.

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.

Muito obrigado pela contribuição.

Favor fazer os ajustes conforme comentários e então fazer um novo commit e solicitar uma nova revisão.

Ah, coloque também o texto closes [link da issue] na descrição do PR. Dessa forma, ao fazermos o merge a issue é fechada automaticamente.

😉

@@ -281,3 +281,23 @@ Cypress.Commands.add('assertStatus', statusText => {
.should('contain', statusText)
cy.get('.qa-user-avatar').click()
})
// CRUD
Cypress.Commands.add('gui_deployKeyCreationName', (name = faker.string.uuid()) => {
cy.visit('http://localhost/admin/deploy_keys/new')
Copy link
Owner

@wlsf82 wlsf82 Nov 30, 2023

Choose a reason for hiding this comment

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

A baseUrl já está definida no arquivo cypress.config.js, e portanto, não precisa estar aqui.

@wlsf82
Copy link
Owner

wlsf82 commented Nov 30, 2023

@Fabricioperrone, hoje fiz mudanças no branch principal que geraram conflitos com o código do teu PR.
Favor ajustar isso também, buscando as mudanças no branch principal do repositório no meu perfil e então fazer um git rebase.

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.

2 participants