frlp-utn-ingsoft / filmster

Aplicación utilizada en la cursada 2019 de Ingenieria de Software. UTN-FRLP
0 stars 11 forks source link

Duda merge dev #51

Closed redBaron23 closed 5 years ago

redBaron23 commented 5 years ago

Hola, 1)Ya hice mi test y anda, deberia hacer el merge a dev ahora, o cuando mi equipo haga el resto? 2)Tengo que hacer un punto en el cual tengo que arreglar esto:

El test incluido en test/unit/ui.test.js falla, demostrando que hay un bug en la app.
Solucionar este bug (para esta tarea, hay que quitar el “skip” incluido al principio del
código del test).

Deberia crear un branch hotFix y hacer como veniamos haciendo, o deberia usar bugFix? de ser bugFix, la conexion seria solo con dev?, ejemplo arreglo el bug desde bugFix y hago:

git checkout dev
git merge bugFix
redBaron23 commented 5 years ago

Hola, 1)Ya hice mi test y anda, deberia hacer el merge a dev ahora, o cuando mi equipo haga el resto? 2)Tengo que hacer un punto en el cual tengo que arreglar esto:

El test incluido en test/unit/ui.test.js falla, demostrando que hay un bug en la app.
Solucionar este bug (para esta tarea, hay que quitar el “skip” incluido al principio del
código del test).

Deberia crear un branch hotFix y hacer como veniamos haciendo, o deberia usar bugFix? de ser bugFix, la conexion seria solo con dev?, ejemplo arreglo el bug desde bugFix y hago:

git checkout dev
git merge bugFix

Con respecto al punto 2 el error es este:

export function parseCSV(val) {
    return val.split(',').flatMap(v => v.split());
}

deberia ser asi?

val.split(', ')

El test es el siguiente deberia el test mandar "item1,item2" o si manda "item1, item2" deberia andar igual?:

const utils = require('../../client/src/utils.mjs');

test('parseCSV', () => {
    const actual = utils.parseCSV('foo, bar')
    const expected = ['foo', 'bar']

    expect(actual).toStrictEqual(expected)
})
RodrigoJacznik commented 5 years ago

Hola. Cuando termines tu test hace el merge a dev.

Con respecto al opcional, los siguientes casos:

-  '         foo, bar'
-  'foo,bar'
-  'foo,    bar'
-  'foo, bar       '

Deberian retornar ['foo', 'bar']. Tu solución no contempla todos estos casos. Podes usar la funcion trim (https://developer.mozilla.org/es/docs/Web/JavaScript/Referencia/Objetos_globales/String/Trim)

Hace un branch bugfix. Los hotfix son solo desde master.

redBaron23 commented 5 years ago

Hola. Cuando termines tu test hace el merge a dev.

Con respecto al opcional, los siguientes casos:

  • ' foo, bar'
  • 'foo,bar'
  • 'foo, bar'
  • 'foo, bar '

Deberian retornar ['foo', 'bar']. Tu solución no contempla todos estos casos. Podes usar la funcion trim (https://developer.mozilla.org/es/docs/Web/JavaScript/Referencia/Objetos_globales/String/Trim)

Hace un branch bugfix. Los hotfix son solo desde master.

Ah perfecto con un replace(" ","") ya andaria no ?

RodrigoJacznik commented 5 years ago

no, creo que hay algunos casos que no los toma. Hace el split como esta ahora y por cada item en el array que te retorna usa trim

redBaron23 commented 5 years ago

no, creo que hay algunos casos que no los toma. Hace el split como esta ahora y por cada item en el array que te retorna usa trim

Te molesto de nuevo, estara bien asi?

export function parseCSV(val) {
    return val.split(',').flatMap(v => v.split()).map(item => item.trim());
}

porque ahora me pasa el test que lo modifique asi:

const utils = require('../../client/src/utils.mjs');

test('parseCSV', () => {
    const actual = utils.parseCSV('  foo  , bar  ')
    const expected = ['foo', 'bar']

    expect(actual).toStrictEqual(expected)
})

Y me tira esto la consola:

image

RodrigoJacznik commented 5 years ago

Si, esta bien resuelto.

El error que te tira es en otro test, actualiza con respecto a filmster o ignoralo (ponele skip despues de test para que no te salte en consola)

redBaron23 commented 5 years ago

Dale , igual se arreglo solo. Osea es medio casual si hago npm run test una vez atras de la otra, a veces pasa sin problemas y otras veces tiene problemas en alguno de estos test: (adjunto otros 2 errores que tiro) image image

Igual ya fue mi test anda, gracias!