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

Organização de pastas dos testes unitários #479

Open
wants to merge 47 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@vitormattos
Copy link
Contributor

vitormattos commented Jan 5, 2019

Descrição

Arquivos movidos de pasta para refletir a estrutura de pastas do projeto.

Um teste estava quebrando pois utilizava sessão, foi preciso fazer um refactor fora da pasta de testes para corrigir e remover a dependência da sessão.

Tipos de alterações

  • Bug fix (Não quebra outras funcionalidades))
  • Breaking change (Correção ou mudança, pode quebrar parte da aplicação)

Checklist:

  • Eu li o documento CONTRIBUTING. [REQUIRED]
  • Meu código segue a PSR2. [REQUIRED]
    • Para evitar dificuldade no code review, isto será feito em outro momento
  • Todos os testes novos e existentes estão passando. [REQUIRED]

vitormattos added some commits Jan 3, 2019

Refletindo a mesma estrutura de pastas do projeto
Remoção de require desnecessário
Refletindo a mesma estrutura de pastas do projeto
Renomeando classe para compatibilidade com nome da classe testada
Remoção de require desnecessário
Renomeando classe para compatibilidade com nome da classe testada
Remoção de require desnecessário
Renomeando classe para compatibilidade com nome da classe testada
Remoção de require desnecessário
Renomeando classe para compatibilidade com nome da classe testada

@edersoares edersoares added the needs-qa label Jan 7, 2019

@edersoares
Copy link
Member

edersoares left a comment

@vitormattos ficou ótima a organização!

Dois pontos:

  • Questão de auditoria e sessão, o i-Educar tem um grande problema que a sessão é fechada em várias partes do sistema, sendo assim a variável global $_SESSION não existe em todo contexto, atentar para este fato.
  • Porque você fez o unset($_GET['etapa']); nos testes, estava gerando problemas, só pra eu entender.
@session_start();
$pessoa_logada = $_SESSION['id_pessoa'];
@session_write_close();

This comment has been minimized.

@edersoares

edersoares Jan 7, 2019

Member

Não é o ideal, mas este trecho não pode ser removido ainda, ele é utilizado para fins de auditoria.

This comment has been minimized.

@edersoares

edersoares Jan 7, 2019

Member

Talvez possa ser colocado dentro de um if para evitar problemas ao rodar os testes, mas precisamos que quando exista sessão a variável $pessoa_logada tenha o ID do usuário.

This comment has been minimized.

@vitormattos

vitormattos Jan 16, 2019

Contributor
  • O unset é porque $_GET é variável global. Nos primeiros testes era definido em um teste valor para esta variável etapa, em testes seguintes o negócio funcionava por conta do primeiro já ter definido mas se utilizarmos filter no phpunit para executar apenas um teste específico que dependesse desta variável etapa que está em $_GET o teste quebrava. Coloquei o unset para isto não ocorrer e corrigi os outros testes que estavam quebrando.

This comment has been minimized.

@vitormattos

vitormattos Jan 16, 2019

Contributor
  • Este trecho de pegar id_pessoa da sessão foi movido para a classe de auditoria e coloquei a lógica que envolve este campo para lá, por isto o removi deste local.
// Seta usuário admin quando não houver usuário pois pode ser API/Novo educação
if (!$this->usuario_id) $this->usuario_id = 1;
if (!$this->usuario_id) {
if(!empty($_SESSION['id_pessoa']) && is_numeric($_SESSION['id_pessoa'])) {

This comment has been minimized.

@edersoares

edersoares Jan 7, 2019

Member

Entendi a remoção no outro local, porém não acredito que um session_start vai precisar ser iniciado aqui, pois a sessão já deve ter sido encerrada em outro local da aplicação (é uma falha conhecida).

This comment has been minimized.

@vitormattos

vitormattos Jan 16, 2019

Contributor

A lógica foi movida do outro arquivo para cá. Por isto tem este acréscimo do condicional aí pois se houver id_pessoa na sessão, vai pegar da sessão, senão, coloca 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment