dorminhoco test + resolution #17

nato-re · 2020-12-04T01:02:44Z

Adiciona exercício junto a gabarito, com intensão de avaliação para construir enunciado

Issue: #11

github-actions

Muito obrigado pela contribuição! Traga mais!

frattezi · 2020-12-05T11:18:11Z

programming-basics/dorminhoco/test.js

+    expect(typeof sonho).toBe('string');
+  });
+
+  it(`resultado deve ser uma string`, async () => {


acho que aqui deveria ser outra mensagem

mara, segue no comitão bomba anexado no email <3

frattezi · 2020-12-05T11:23:34Z

Vamos pensar em como deixar este exercise apenas no template inicial + Uma historinha legal. Vendo o código eu pensei que poderiamos ter uma lista fixa que o user não pode alterar a forma e deve printar cada item em ordem, porém, exigimos uma ordem de impressão diferente da ordem da lista? Não sei ainda não consigo ver esse danado como exercício.

frattezi

Boa! Vamos conversando e vendo como melhorar

frattezi · 2020-12-05T11:24:28Z

programming-basics/dorminhoco/main.js

+  'Mozão'
+]
+const randomPosition = Math.floor(Math.random() * dreams.length)
+const randomDream = (resolve) => () => resolve(dreams[randomPosition])


sempre tentar manter um pattern, se aqui as consts estão em inglês isto deveria ser seguido em todo code, para um lado ou pro outro

programming-basics/dorminhoco/test.js

programming-basics/dorminhoco/main.js

nato-re · 2020-12-07T02:33:33Z

programming-basics/dorminhoco/test.js

-    expect(setTimeout).toBeCalledWith(expect.any(Function), 2000)
-
-
+    expect(setTimeout).toBeCalledWith(expect.any(Function), 42);


função chamada com undefined, não entendi o processo

seu teste não é async, acredito que isto está impactando sua leitura

faz mt sentido :D
vou implementar!

nato-re · 2020-12-07T02:34:18Z

programming-basics/dorminhoco/main.js

@@ -1,24 +1,21 @@
-const dreams = [
+const dreamList = [


ainda estou pensando em como fazer com a lista sem aleatoriaedade

acho que fica mais fácil. talvez, vamos tentar moldar a ordem do problema de uma vez antes de ir para implementação

aleatoriedade é o que torna os testes mais fracos, dado que você pode ter resultados diferentes para cada vez que vc rodar os testes, acho que seria bom simplificar essa implementação mesmo

Uma possibilidade é mockar a função 'Math.random()'

muito bem, vou implementar, junto com os testes dos caminhos feios aushda <3

marco-souza · 2020-12-12T13:32:18Z

programming-basics/dorminhoco/main.js

@@ -1,24 +1,21 @@
-const dreams = [
+const dreamList = [


Uma possibilidade é mockar a função 'Math.random()'

marco-souza · 2020-12-12T13:34:07Z

programming-basics/dorminhoco/main.js

+const sleep = (ms, cb) => setTimeout(cb, ms);
+
+const dorminhoco = async (milliseconds, dreamList) => {
+  const promisedDream = new Promise((resolve) =>


A gente não precisa disso.

vc ta misturando callback com async/await, sendo que ambos server ao mesmo proposito, fazer uma chamada no asincrona.

recomendo:

remover o cb do sleep

chamar a função randomDream depois de um await

fiz no commit, acho que ficou de acordo com suas colocações

marco-souza · 2020-12-12T13:37:32Z

programming-basics/dorminhoco/main.js

+const sleep = (ms, cb) => setTimeout(cb, ms);
+
+const dorminhoco = async (milliseconds, dreamList) => {
+  const promisedDream = new Promise((resolve) =>
+    sleep(milliseconds, randomDream(dreamList, resolve))
+  );
+  const dream = await promisedDream;


Suggested change

const sleep = (ms, cb) => setTimeout(cb, ms);

const dorminhoco = async (milliseconds, dreamList) => {

const promisedDream = new Promise((resolve) =>

sleep(milliseconds, randomDream(dreamList, resolve))

);

const dream = await promisedDream;

const sleep = (ms) => new Promise(res => setTimeout(cb, res);

export const dorminhoco = async (milliseconds, dreamList) => {

await sleep(milliseconds)

const dream = randomDream()

gostei, implementei parecido no commit

marco-souza · 2020-12-12T13:38:52Z

programming-basics/dorminhoco/test.js

@@ -0,0 +1,26 @@
+const main = require('./main');


vamos evitar require em nossos projetos em pro da sintaxe mais recente e recomendada de import { dorminhoco } from './main'

marco-souza · 2020-12-12T13:39:00Z

programming-basics/dorminhoco/main.js

+  return dream;
+};
+
+module.exports = { dorminhoco };


Vamos evitar esse export

marco-souza · 2020-12-12T13:41:07Z

programming-basics/dorminhoco/test.js

+    jest.clearAllMocks();
+    jest.useFakeTimers();
+  });
+  const dreamMock = ['não lembro do sonho'];


Declare essa variavel fora do describe, por padrão, geralmente declaramos constantes imutáveis no escopo raiz como uma boa prática

marco-souza · 2020-12-12T13:43:11Z

programming-basics/dorminhoco/test.js

+
+    expect(setTimeout).toBeCalledWith(expect.any(Function), 42);
+  });
+});


Não temos casos de testes com listas vazias.

quando fazemos unit teste temos que validar caminhos felizes e caminhos tristes, como por exemplo:

lista vazia

string no lugar da lista

lista undefined

so on

Acho válido, mas estou tentando testar com o node assim:
dorminhoco(42, [])
e dá isso:
SyntaxError: Unexpected token 'export'

node por padrão não reconhece o exoprt, precisamos do babel para tal. No entanto, o Jest deveria lidar com isso pra gente.

O ideal é rodar com jest e ir testando

marco-souza · 2020-12-12T13:43:41Z

programming-basics/dorminhoco/test.js

-    expect(setTimeout).toBeCalledWith(expect.any(Function), 2000)
-
-
+    expect(setTimeout).toBeCalledWith(expect.any(Function), 42);


marco-souza · 2020-12-12T13:44:38Z

programming-basics/dorminhoco/test.js

+  it(`setTimeout dever ser sido chamado uma vez`, async () => {
+    main.dorminhoco(42, dreamMock);
+
+    expect(setTimeout).toBeCalledTimes(1);


não estamos validando a função setTimeout, ela já é testada pelo core team do JS.

O que queremos testar é a função dorminhoco e seus retornos

marco-souza · 2020-12-12T13:46:38Z

programming-basics/dorminhoco/test.js

+  });
+  const dreamMock = ['não lembro do sonho'];
+
+  it(`resultado deve ser uma string`, async () => {


Este caso de teste é um bom exemplo de caminho feliz, agora precisamos fazer os testes para caminhos triste

marco-souza

Bom trabalho, a função já ta bem melhor! 👍

Alguns pontos de atenção:

não devemos enviar a resolução do exercicio, apenas os testes, a descrição e algumas funções uteis para resolver o problema
Os nomes dos testes precisam ser mais descritivos, pois é onde os estudantes da podcodar vão validar suas resoluções.
Precisamos de um padrão de nome para todos os casos de teste para facilitar a leitura de quem for resolver
Temos de prestar atenção nos asserts, para garantirmos que estamos validando o que queremos de fato, e não fazendo validacões que envolvem transformação/coerção de tipos, e funções nativas do JS.

PS: foi mal os longos reviews, não é nada pessoal, hahaha, só estou tentando manter o exercicio dentro dos nossos padrões (que ainda precisam ser melhor definidos, mas já conseguimos dar dicas de boas práticas e tals...) 😃

programming-basics/dorminhoco/test.js

marco-souza · 2020-12-25T10:50:28Z

programming-basics/dorminhoco/test.js

+
+    expect(setTimeout).toBeCalledWith(expect.any(Function), 42);
+  });
+});


node por padrão não reconhece o exoprt, precisamos do babel para tal. No entanto, o Jest deveria lidar com isso pra gente.

programming-basics/dorminhoco/test.js

marco-souza · 2020-12-25T10:53:27Z

programming-basics/dorminhoco/test.js

+    expect(setTimeout).toBeCalledWith(expect.any(Function), 42);
+  });
+
+  it(`checa se sonho tem retorno válido`, async () => {


Acho que os nomes dos casos de testes precisam ser mais descritivos como "Quando a lista de sonhos é vazia, o retorno deve ser inválido" ou algo assim

Isso ainda não foi implementado, mas já está em mente

programming-basics/dorminhoco/test.js

marco-souza · 2020-12-25T10:55:59Z

programming-basics/dorminhoco/test.js

+
+    expect(setTimeout).toBeCalledWith(expect.any(Function), 42);
+  });
+});


O ideal é rodar com jest e ir testando

marco-souza

Boa, estamos chegando lá! 🐱

Próximos passos ao meu ver seriam

sugestões da revisão atual
remover resolução da main.js
Adicionar descrição do problema no README.md
rodar prettier e/ou eslint para mantar os estilos homogêneos
melhor descrição dos testes
Refatorar os commits com git rebase -i para entrar no padrão definido pelo @frattezi

marco-souza · 2021-01-03T15:37:52Z

programming-basics/dorminhoco/test.js

    jest.clearAllMocks();
-    jest.useFakeTimers();
+    jest.restoreAllMocks();
+    jest.spyOn(global.Math, 'random').mockReturnValue(0);


Boa, mandou bem no mock!

Uma dúvida: precisamos de dar spyOn sempre antes de cada test case? Ou podemos usá-lo apenas uma vez e aceitar que o jest.clearAllMocks() vai limpar o mock pra gente?

marco-souza · 2021-01-03T15:44:46Z

programming-basics/dorminhoco/test.js

+  });
+
+  it(`resultado deve ser uma string`, () => {
+    return dorminhoco(42, dreamMock).then((dream) => {


Acho que seria mais elegante usar async/await para lidar com promises

marco-souza · 2021-01-03T15:45:50Z

programming-basics/dorminhoco/test.js

+
+  it(`resultado deve ser uma string`, () => {
+    return dorminhoco(42, dreamMock).then((dream) => {
+      expect(typeof dream).toBe('string');


Não precisamos validar o type, dado que estamos validando o valor + tipo logo abaixo

marco-souza · 2021-01-03T15:47:57Z

programming-basics/dorminhoco/test.js

+  });
+
+  it(`checa retorno com entrada como string`, () => {
+    return dorminhoco(42, 'nao sou um array').then((dream) => {


me parece estranho rodar com uma string. Não deveriamos rodar apenas com array? Mesmo o JS tratando Array como String, acho que seria melhor nao rodar para strings

# input: 42 "minha string' # output undefined

marco-souza · 2021-01-03T15:51:00Z

programming-basics/dorminhoco/test.js

+
+  it(`espera que setTimeout seja chamado com uma função`, async () => {
+    jest.useFakeTimers();
+    dorminhoco(42, dreamMock);


dado que o dominhoco é uma promise, precisamos de dar await aqui para aguardar a execução. Do jeito que está vai passar por causa do asset feito abaixo, que só valida se setTimeout foi chamado, mas num teste real, precisamos aguardar as promisses se resolverem antes de validar o retorno

marco-souza · 2021-01-03T15:54:47Z

programming-basics/dorminhoco/main.js

+
+  await sleep(milliseconds)
+
+  const dream = randomDream(dreamList);
+  return dream;


Suggested change

await sleep(milliseconds)

const dream = randomDream(dreamList);

return dream;

if (Array.isArray(dreamList))

return // early return to avoid extra processing

await sleep(milliseconds)

return randomDream(dreamList)

Simplificando a implementação

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray talvez seja bom mencionar isso na descrição do problema

dorminhoco test + resolution

5fb96cb

nato-re added the exercício label Dec 4, 2020

nato-re requested a review from frattezi December 4, 2020 01:02

nato-re self-assigned this Dec 4, 2020

github-actions bot reviewed Dec 4, 2020

View reviewed changes

nato-re added this to the V1 milestone Dec 4, 2020

frattezi reviewed Dec 5, 2020

View reviewed changes

marco-souza requested changes Dec 7, 2020

View reviewed changes

deal with CR sugestions and try to aply them, problem with 3 test

cd7b2e4

nato-re commented Dec 7, 2020

View reviewed changes

marco-souza requested changes Dec 12, 2020

View reviewed changes

nato-re added 2 commits December 21, 2020 16:29

implement CR suggestions

e5ff4c7

implement CR suggestions

88f7d0f

marco-souza requested changes Dec 25, 2020

View reviewed changes

nato-re added 2 commits December 30, 2020 16:54

run tests with warning 'jest.runAllTimers() issue reported'

0fa7d0d

agora foi, xablau

c4eba61

marco-souza requested changes Jan 3, 2021

View reviewed changes

		expect(setTimeout).toBeCalledWith(expect.any(Function), 2000)


		expect(setTimeout).toBeCalledWith(expect.any(Function), 42);

dorminhoco test + resolution #17

Are you sure you want to change the base?

dorminhoco test + resolution #17

Conversation

nato-re commented Dec 4, 2020

github-actions bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frattezi commented Dec 5, 2020

frattezi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marco-souza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marco-souza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment