Skip to content

Chris d'Arcy Node Eval#178

Open
cmdarcy wants to merge 20 commits into
projectshft:masterfrom
cmdarcy:master
Open

Chris d'Arcy Node Eval#178
cmdarcy wants to merge 20 commits into
projectshft:masterfrom
cmdarcy:master

Conversation

@cmdarcy

@cmdarcy cmdarcy commented Apr 13, 2025

Copy link
Copy Markdown

No description provided.

Comment thread test/server.test.js
// create constants for tests
const validUser = { userName: 'yellowleopard753', password: 'jonjon' };
const user = users.find((u) => u.login.username === validUser.userName);
const invalidUsers = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

always include the word mock as these are not real

Suggested change
const invalidUsers = [
const invalidUsersMock = [

Comment thread test/server.test.js
{ password: 'valid' },
{},
];
const incorrectUsers = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

spacing

Comment thread test/server.test.js
describe('/GET brands/:brandId/products', () => {
it('should respond with 404 status when the supplied brand does not exist', (done) => {
const brandId = '7';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

space

Comment thread test/server.test.js
});
});

it('should treat an empty searchTerm the same as not supplying one', (done) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not clear statement

Comment thread test/server.test.js
res.should.have.status(200);
res.body.should.be.an('array');
res.body.should.not.be.empty;
res.body[0].should.have.all.keys(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you are not really testing here the search functionality, you need to make sure the actual data is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants