Feature reamining-test #10
Conversation
edenriquez
force-pushed
the
feature/remaining-test
branch
2 times, most recently
from
3da3cff
to
0196c92
Compare
| ord, _ := CreateOrderWithoutCharges() | ||
| _, err := Find(ord.ID, "123") | ||
| assert.NotNil(t, err) | ||
| assert.Equal(t, err.ErrorType, "resource_not_found_error") |
There was a problem hiding this comment.
Aquí no podríamos evaluar algo así como len(ord.Charges.Data) == 0 ?? para validar que esa orden no tenga nada
There was a problem hiding this comment.
Realmente creo que no tiene caso revisar que la orden no tenga cargos ya que en le mock no estoy incluyendo el objeto de cargos por que el test asegura que la función Find maneje la excepción de forma adecuada, no se que opinas ?
charge/client.go
Outdated
| @@ -8,13 +8,13 @@ import ( | |||
| //For more information please see https://developers.conekta.com/api#create-charge | |||
| func Create(id string, p *conekta.ChargeParams) (*conekta.Charge, *conekta.Error) { | |||
There was a problem hiding this comment.
Creo que sería mejor cambiar el nombre del parámetro id por ordID o algo así porque es confuso a qué hace ID hace referencia esa función porque luego en el find se usa Find(orderID string, id string) y ahí pensaría que es como el id del charge o algo así
There was a problem hiding this comment.
si de hecho el nombre id quiere dar a entender que es el identificador asociado al paquete es decir si estoy en charge las funciones que reciban id string serán los id's de dichos objetos y únicamente distingo los ids de otros objetos con su nombre por ejemplo el de orderID no se si se entiende el propósito o sugieres que ponga chargeID
charge/client.go
Outdated
| ch := &conekta.Charge{} | ||
| err := conekta.MakeRequest("GET", "/orders/"+id+":/charges/"+id, p, ch) | ||
| err := conekta.MakeRequest("GET", "/orders/"+id+"/charges/"+id, &conekta.EmptyParams{}, ch) |
There was a problem hiding this comment.
Creo que aquí se te fue un id, el primero debe ser el del order
"/orders/"+ orderID +"/charges/"+id
discountlines/client.go
Outdated
| // Delete deletes a Order Discount Line | ||
| func Delete(id string) (*conekta.DiscountLines, *conekta.Error) { | ||
| ord := &conekta.DiscountLines{} | ||
| err := conekta.MakeRequest("DELETE", "/orders/"+id, &conekta.OrderParams{}, ord) |
There was a problem hiding this comment.
Aquí no tendría que ser algo cómo
"/orders/"+orderID+"/discount_lines/"+id ?? porque me parece que ahí estás tratando de eliminar la orden
There was a problem hiding this comment.
si, lo corrijo y agrego test's
| return ord, err | ||
| } | ||
|
|
||
| func TestCreate(t *testing.T) { |
There was a problem hiding this comment.
Aquí me gustaría si podemos hacer 2 test create, CreateWitouthParams y el otro create y para el de sin parámetros validar algo como len(ord.DiscountLines.Data) == 0
There was a problem hiding this comment.
Creo que un data con longitud 0 no es posible, el test de CreateError le manda un DiscountLineParams sin argumentos pero la validación del api necesita argumentos para crear la linea de descuento por ende una linea no puede quedar vacía.
| @@ -23,14 +24,15 @@ type Order struct { | |||
| Currency string `json:"currency,omitempty"` | |||
| CreatedAt int `json:"created_at,omitempty"` | |||
| UpdatedAt int `json:"updated_at,omitempty"` | |||
| PreAuth bool `json:"pre_authorize,omitempty"` | |||
There was a problem hiding this comment.
Está duplicado este no?? veo 2 veces
PreAuth bool json:"pre_authorize,omitempty"
There was a problem hiding this comment.
si pero para una orden pre autorizada se puede mandar como Param y tambien puedes recibir el flag PreAuth por eso esta tanto en OrderParams como en Order
requestor.go
Outdated
| @@ -16,6 +16,9 @@ func RequestAPI(method string, url string, params ParamsConverter) ([]byte, *Err | |||
| req, _ := http.NewRequest(method, requestURL, bytes.NewBuffer(params.Bytes())) | |||
|
|
|||
| setHeaders(req) | |||
| if url == "/tokens" { | |||
There was a problem hiding this comment.
Creo que hay que poner un TODO aquí porque lo siento muy hardcode
edenriquez
force-pushed
the
feature/remaining-test
branch
from
0196c92
to
2b2aa03
Compare
edenriquez
force-pushed
the
feature/remaining-test
branch
from
2b2aa03
to
afd8399
Compare
Why is this change neccesary?
Adding remaining tests we assurance the right behaviour of the model
also almost all structs are assured and checked that already works with api response
How does it address the issue?
were created all model/submodels mocks and his respective unit test
What side effects does this change have?
nothing
How to test it?
running tests