-
Notifications
You must be signed in to change notification settings - Fork 0
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
Test worker priorization #45
base: master
Are you sure you want to change the base?
Conversation
type dbInterface interface { | ||
GetPipelinesByDay(day int) ([]storage.Pipeline, error) | ||
InsertExecution(e storage.Execution) error | ||
GetLastExecutionsForAllPipelines() ([]storage.Execution, error) |
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.
Você provavelmente vai precisar passar algum tipo de quantidade aqui, não?
Outra coisa importante, a interface pode ser um bom lugar para documentar bem essas funções. Por exemplo, o método GetPipelinesByDay(int)
retorna todos os pipelines do dia especificado, sempre no mês corrente.
@@ -22,6 +29,16 @@ func main() { | |||
log.Fatal("error trying get environment variable: $MONGODB is empty") | |||
} | |||
|
|||
errorLimitStr := os.Getenv("ERROR_LIMIT") |
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.
ERROR_LIMIT
é a quantidade de vezes que a execução de um determinado mês pode falhar? Se sim, poderia chamar ela RETRY_LIMIT
, por favor? Essa é a forma que ela se apresenta em outros programas.
Também poderia comentar essa variável? Vale super pena documentar esse parâmetro do programa.
@@ -22,6 +29,16 @@ func main() { | |||
log.Fatal("error trying get environment variable: $MONGODB is empty") | |||
} | |||
|
|||
errorLimitStr := os.Getenv("ERROR_LIMIT") | |||
if uri == "" { |
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.
if uri == "" { | |
if errorLimitStr == "" { |
@@ -22,6 +29,16 @@ func main() { | |||
log.Fatal("error trying get environment variable: $MONGODB is empty") | |||
} | |||
|
|||
errorLimitStr := os.Getenv("ERROR_LIMIT") | |||
if uri == "" { |
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.
Lembrando que o strconv.Atoi
falha quando a variável é vazia. Mas entendo que você quis dar uma mensagem mais bacana quando o erro mais recorrente.
return list | ||
} | ||
|
||
func getPipelinesToExecuteToday(db *storage.DBClient) ([]storage.Pipeline, error) { | ||
func getPipelinesToExecuteToday(db dbInterface) ([]storage.Pipeline, error) { |
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 acho arretada essa forma que programas em Go crescem em complexidade. Hoje em dia me soa bastante orgânico, natural? Nenhuma modição em extends ou implements! Note que você não precisou modificar em nada storage.DBClient
!
}, | ||
} | ||
|
||
type fakeFinder struct { |
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.
fakeDB?
"github.com/matryer/is" | ||
) | ||
|
||
var pipelinesDB = []storage.Pipeline{ |
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.
Tu deve precisar criar vários desses a medida que o teste aumenta.
Sugestão: cria os storage.Pipeline
baseado nas características que você quer, coloca nomes sugestivos neles (dada as características de erro ou algo simples ou complexo ou algo assim) e coloca eles na parte de baixo do programa.
Daí, cria o []storage.Pipeline
dentro de cada método. A pessoa lendo esse arquivo ver os casos de teste e quando for ler um terminado método vai entender mais fácilmente o caso sendo testado.
// 2º descartar se o número de execuções com erro for igual ou maior que o limite | ||
// ... | ||
// por último aplicar filtro de tamanho | ||
func TestPrioritizeAndLimit_LimitTest(t *testing.T) { |
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.
Para poder pensar nas 2 funções getPipelinesThatFailed e getPipelinesForCompleteHistory precisei pensar no processo como um todo a fim de deixar essas funções "mais simples", e então centralizar as regras que queremos aplicar no prioritizeAndLimit. Por isso fiz essa primeira versão de teste, o que me ajudou a pensar nas funções novas no storage também =].
O próximo passo é implementar as funções no storage para depois fazer o getPipelinesThatFailed.
E acredito que esse processo deve gerar algum ajuste nas ideias descritas aqui para a priorização.