Skip to content

Increase Coverage#360

Merged
johnanvik merged 56 commits intomasterfrom
testingCoverage
Jul 4, 2018
Merged

Increase Coverage#360
johnanvik merged 56 commits intomasterfrom
testingCoverage

Conversation

@JaceRiehl
Copy link
Collaborator

@JaceRiehl JaceRiehl commented Jun 26, 2018

Increases the test coverage.

@coveralls
Copy link

coveralls commented Jun 28, 2018

Pull Request Test Coverage Report for Build 1025

  • 24 of 46 (52.17%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+15.8%) to 23.486%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/SharedComponents/SettingsComponent.vue 0 1 0.0%
src/classes/AiActions/Personality.js 0 21 0.0%
Totals Coverage Status
Change from base Build 970: 15.8%
Covered Lines: 510
Relevant Lines: 1834

💛 - Coveralls

@johnanvik
Copy link
Collaborator

Based on the title, I'm unclear if this is actually submitted for review. I'll wait until you assign this to me as a reviewer as the signal that you want this merged in.

@JaceRiehl JaceRiehl requested a review from johnanvik July 3, 2018 15:33
@JaceRiehl JaceRiehl changed the title Test build Increase Coverage Jul 3, 2018
Copy link
Collaborator

@johnanvik johnanvik left a comment

Choose a reason for hiding this comment

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

I have some style comments, but these are minor.

I'm trying to run the tests locally, but they are currently failing:

image

Will be looking more into this to see if the issue with the pull request or my environment.

@@ -0,0 +1,82 @@
// var chai = require('chai');
var sinon = require('sinon')
import {store} from '../../../src/store/store'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Absolute Import references like these make me nervous. If the directory structure gets changed in the future, this will break.

Having said that though, I'm not sure the likelihood of directory structure changes in the future.

let oPO = player2
let oV = player2
let aVMock = new AntiVirus(hand, boolSide, move, event)
// let aVSpy = sinon.spy(aVMock, 'execute')
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 there is some dead code to clean up here.

store.state.players = []
expect(store.getters.maxplayers).to.equal(0)
})
it('test the store currentPlayerName function', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding spaces between the tests to improve readability.

// })
import MainComponent from '../../../src/components/MainGame/MainComponent.vue'

// function getSubmit (Component, propsData) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dead code

expect(defaultData.winnerScore).to.equal(0)
expect(Array.isArray(defaultData.deleteData)).to.equal(true)
})
// it('testing the submit funtion', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dead code

store.commit('resetState')
expect(store.getters.getWinner).to.equal(false)
})
it('test the store addPlayers function', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

See the previous comment about readability.

})
it('test the setPlayfieldColour function', () => {
store.commit('setPlayfieldColour', false)
// expect(store.state.trueSideColour).to.equal('background-color: #80aef7; box-shadow: 0px 3px 15px rgba(0,0,0,0.6)')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dead code

@johnanvik
Copy link
Collaborator

The problem I was having running the tests was fixed by npm update.

@johnanvik johnanvik merged commit b5f9cab into master Jul 4, 2018
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