-
Notifications
You must be signed in to change notification settings - Fork 13
dorminhoco test + resolution #17
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
base: master
Are you sure you want to change the base?
Changes from all commits
5fb96cb
cd7b2e4
e5ff4c7
88f7d0f
0fa7d0d
c4eba61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,19 @@ | ||||||||||||||||||||
const dreamList = [ | ||||||||||||||||||||
'Ovelhas astronautas', | ||||||||||||||||||||
'Tava jogando mesmo', | ||||||||||||||||||||
'Sonhei com código', | ||||||||||||||||||||
'Mozão', | ||||||||||||||||||||
]; | ||||||||||||||||||||
|
||||||||||||||||||||
const randomDream = (dreams) => | ||||||||||||||||||||
dreams[Math.floor(Math.random() * dreams.length)]; | ||||||||||||||||||||
|
||||||||||||||||||||
const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms)); | ||||||||||||||||||||
|
||||||||||||||||||||
nato-re marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||
export const dorminhoco = async (milliseconds, dreamList) => { | ||||||||||||||||||||
|
||||||||||||||||||||
await sleep(milliseconds) | ||||||||||||||||||||
|
||||||||||||||||||||
const dream = randomDream(dreamList); | ||||||||||||||||||||
return dream; | ||||||||||||||||||||
Comment on lines
+14
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplificando a implementação There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray talvez seja bom mencionar isso na descrição do problema |
||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { dorminhoco } from './main'; | ||
const dreamMock = ['não lembro do sonho', 'test', 'test2' ]; | ||
|
||
describe('Testes - Dominhoco', () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
jest.restoreAllMocks(); | ||
jest.spyOn(global.Math, 'random').mockReturnValue(0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Boa, mandou bem no mock! Uma dúvida: precisamos de dar |
||
}); | ||
|
||
it(`checa retorno com array vazio como entrada`, () => { | ||
return dorminhoco(42, []).then((dream) => { | ||
expect(dream).toBe(undefined); | ||
}); | ||
}); | ||
|
||
it(`resultado deve ser uma string`, () => { | ||
return dorminhoco(42, dreamMock).then((dream) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acho que seria mais elegante usar async/await para lidar com promises |
||
expect(typeof dream).toBe('string'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Não precisamos validar o type, dado que estamos validando o valor + tipo logo abaixo |
||
expect(dream).toBe(dreamMock[0]); | ||
}); | ||
}); | ||
|
||
it(`checa retorno com entrada falsa`, () => { | ||
return dorminhoco(42, false).then((dream) => { | ||
expect(dream).toBe(undefined); | ||
}); | ||
}); | ||
|
||
it(`checa retorno com entrada como numero`, () => { | ||
return dorminhoco(42, 42).then((dream) => { | ||
expect(dream).toBe(undefined); | ||
}); | ||
}); | ||
marco-souza marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
it(`checa retorno com entrada como string`, () => { | ||
return dorminhoco(42, 'nao sou um array').then((dream) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||
expect(dream).toBe('n'); | ||
}); | ||
}); | ||
|
||
it(`espera que setTimeout seja chamado com uma função`, async () => { | ||
jest.useFakeTimers(); | ||
dorminhoco(42, dreamMock); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. dado que o dominhoco é uma promise, precisamos de dar |
||
expect(setTimeout).toBeCalledWith(expect.any(Function), 42); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. função chamada com undefined, não entendi o processo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seu teste não é async, acredito que isto está impactando sua leitura There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. faz mt sentido :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
}); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Não temos casos de testes com listas vazias. quando fazemos unit teste temos que validar caminhos felizes e caminhos tristes, como por exemplo:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acho válido, mas estou tentando testar com o node assim: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. node por padrão não reconhece o There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. O ideal é rodar com jest e ir testando |
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 estou pensando em como fazer com a lista sem aleatoriaedade
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 mais fácil. talvez, vamos tentar moldar a ordem do problema de uma vez antes de ir para implementaçã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.
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
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.
Uma possibilidade é mockar a função 'Math.random()'
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.
+1
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.
muito bem, vou implementar, junto com os testes dos caminhos feios aushda <3