Skip to content

Some basic tests for answer route and model#123

Merged
DarkPurple141 merged 3 commits intodevfrom
s3-answer-tests
Sep 6, 2018
Merged

Some basic tests for answer route and model#123
DarkPurple141 merged 3 commits intodevfrom
s3-answer-tests

Conversation

@NunoDasNeves
Copy link
Collaborator

No description provided.

@coveralls
Copy link

coveralls commented Sep 5, 2018

Pull Request Test Coverage Report for Build 262

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.655%

Totals Coverage Status
Change from base Build 241: 0.0%
Covered Lines: 173
Relevant Lines: 185

💛 - Coveralls

Copy link
Collaborator

@DarkPurple141 DarkPurple141 left a comment

Choose a reason for hiding this comment

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

Also these are all in the wrong file. They're testing the routes. See routes folder for this sort of testing.

.set('Accept', 'application/json')
.expect('Content-Type', /json/)
.expect(400)
.then(response => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like body does actually exist.

.expect('Content-Type', /json/)
.expect(200)
.then(response => {
assert(response.body.length === 2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

change to two separate tests described as in slack. (two asserts implies two it's)

Copy link
Collaborator

@DarkPurple141 DarkPurple141 left a comment

Choose a reason for hiding this comment

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

Then all g.

const expect = chai.expect

/* TODO build out tests to also include catching expected JSON response */
describe('Question route testing', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thus should still exist.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrapping what you've done below.

@NunoDasNeves
Copy link
Collaborator Author

ok review plz

const app = require('../../src')
const supertest = require('supertest')(app)
const chai = require('chai')
const expect = chai.expect
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be:

const { expect } = require('chai')

@DarkPurple141 DarkPurple141 merged commit 0ee5d74 into dev Sep 6, 2018
@DarkPurple141 DarkPurple141 deleted the s3-answer-tests branch September 6, 2018 02:18
@DarkPurple141 DarkPurple141 mentioned this pull request Sep 6, 2018
5 tasks
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.

3 participants