Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix track tests #120

Merged
merged 19 commits into from
May 15, 2017
Merged

Fix track tests #120

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
bfac0f6
Add entry and table info to logging message
miporto May 12, 2017
4356f7f
Add ids to test artists
miporto May 12, 2017
44c5bbc
Add ids to tests tracks and use valid artists ids
miporto May 12, 2017
f99eb32
Add real artists before adding a track
miporto May 12, 2017
1f1c1b0
Check req body key: tracks length in get
miporto May 12, 2017
5b7be04
Add artists short body
miporto May 12, 2017
f7a7998
Test existence of artists in get track by id
miporto May 12, 2017
072109a
Change Album and Artists response formatters
miporto May 12, 2017
999c42f
Typo
miporto May 12, 2017
c202c29
Check that artists exist before creating the track
miporto May 12, 2017
be221b6
Set logging of tables creation to debug
miporto May 12, 2017
1d33461
Unify response formatter of track endpoint.
miporto May 12, 2017
b1c03b3
Use valid artists ids in test.
miporto May 12, 2017
8970fbd
Ensure artist are created before tracks
miporto May 12, 2017
1466f34
Add return to promise inside promise's then
miporto May 12, 2017
de60743
Only map artists if they exist.
miporto May 12, 2017
eaae7dd
Fix indentation
miporto May 12, 2017
c208f40
Create valid test artists before creating test track
miporto May 12, 2017
16aeaac
Separate artist and album find functions to run them in parallel
miporto May 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/handlers/db/generalHandler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const findAllEntries = (tableName) => {
};

const findEntryWithId = (tableName, id) => {
logger.info(`Searching for entry ${id}`);
logger.info(`Searching for entry ${id} in ${tableName}`);
return db(tableName).where('id', id).first('*');
};

Expand All @@ -33,7 +33,7 @@ const findAllWithAttributes = (tableName, attributes) => {
};

const createNewEntry = (tableName, entry) => {
logger.info('Creating entry');
logger.info(`Creating entry ${JSON.stringify(entry, null, 4)} in ${tableName}'s table.`);
return db(tableName).insert(entry).returning('*');
};

Expand Down
10 changes: 10 additions & 0 deletions src/test/routes/artist.constants.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
{
"initialArtist": {
"id": 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Estos ids los tenemos en varios pero creo que no esta bueno. No estoy segura de si se insertan asi o si no y despues los usamos asumiendo que quedaron con ese id. Nos complica mucho si guardamos las entries que devuelve knex una ve que creó y usamos ese id?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si le agregas el id y el mismo no esta repetido o es invalido la entry va a tener ese id seguro. Si queremos usar el id que devuelve knex independientemente de su valor habria que borrar esos campos de los json y no determinar un id valido de antemano en los json.

"name": "Papa Baute",
"popularity": 2
},
"initialShortArtist": {
"id": 1,
"name": "Papa Baute"
},
"testArtist": {
"id": 2,
"name": "Luis Fonsi",
"popularity": 1
},
"testShortArtist": {
"id": 2,
"name": "Luis Fonsi"
},
"trackTestArtist": {
"name": "Cage The Elephant",
"popularity": 3
Expand Down
4 changes: 3 additions & 1 deletion src/test/routes/track.constants.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
"id": 24
},
"initialTrack": {
"id": 1,
"name": "Despacito",
"albumId": 240,
"artists": [4, 1]
"artists": [1, 2]
},
"testTrack": {
"id": 2,
"name": "Come a Little Closer",
"albumId": 229,
"artists": [20, 9]
Expand Down
28 changes: 25 additions & 3 deletions src/test/routes/track.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const jwt = require('jsonwebtoken');
const request = require('supertest');
const chai = require('chai');
const chaiHttp = require('chai-http');
const logger = require('../../utils/logger');

chai.should();
chai.use(chaiHttp);
Expand All @@ -25,7 +26,23 @@ describe('Track', () => {
.then(() => {
db.migrate.latest()
.then(() => {
dbHandler.track.createNewTrackEntry(constants.initialTrack)
const createArtists = () => {
dbHandler.general.createNewEntry(tables.artists,
[
artistsConstants.initialArtist,
artistsConstants.testArtist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si va a ser un initial siempre metele initialArtist2

]
)
.then((artists) => logger.debug(`Tests Artists created: ${JSON.stringify(artists, null, 4)}`))
.catch((error) => logger.warn(`Test Artists creation Error: ${error}`));;
};
const createTracks = () => {
dbHandler.track.createNewTrackEntry(constants.initialTrack)
.then((tracks) => logger.debug(`Tests Tracks created: ${JSON.stringify(tracks, null, 4)}`))
.catch((error) => logger.warn(`Test Tracks creation Error: ${error}`));
};
const entryCreators = [createArtists(), createTracks()];
Promise.all(entryCreators)
.then(() => done())
.catch(error => done(error));
})
Expand Down Expand Up @@ -60,6 +77,7 @@ describe('Track', () => {
res.body.metadata.should.have.property('count');
res.body.should.have.property('tracks');
res.body.tracks.should.be.a('array');
res.body.tracks.should.have.lengthOf(1);
done();
});
});
Expand Down Expand Up @@ -166,8 +184,12 @@ describe('Track', () => {
res.body.track.should.have.property('name').eql(constants.initialTrack.name);
res.body.track.should.have.property('duration');
res.body.track.should.have.property('href');
res.body.track.should.have.property('album'); // TODO
res.body.track.should.have.property('artists');
res.body.track.should.have.property('album');
res.body.track.should.have.property('artists')
.eql([
artistsConstants.initialShortArtist,
artistsConstants.testShortArtist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aca! mejor comparar contra lo que devovio knex y no contra la constante que suponemos que va a devolver, no?

]);
res.body.track.should.have.property('popularity');
// TODO add check for 'rate: int' inside popularity object
done();
Expand Down