Skip to content

Pr pour correction groupe room #5#2

Open
wilsim888 wants to merge 13 commits intodannyforest:masterfrom
wilsim888:Carlus
Open

Pr pour correction groupe room #5#2
wilsim888 wants to merge 13 commits intodannyforest:masterfrom
wilsim888:Carlus

Conversation

@wilsim888
Copy link

No description provided.

cy.get('input.input-field').type('Michel')
cy.get('#startOptions > :nth-child(2)').select('Geography')
cy.get('.start-button').click()
cy.get('.choices > :nth-child(1)').click()
Copy link
Owner

Choose a reason for hiding this comment

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

amélioration: loop 5 tours

Copy link
Owner

@dannyforest dannyforest left a comment

Choose a reason for hiding this comment

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

À cause des commentaires dans LocalStorage.spec.js, j'ai dû compter 4 tests qu'on n'étant pas bons. Donc j'ai dû enlever 4 tests non-fonctionnels.

Le total est donc de 4/5 pour les tests unitaires.

Note finale:
Section 1: 4/5
Section 2: 4/5
Section 3: 5/5
Section 4: 5/5

Total: 18/20

cy.get('.add-question-button').then(($button) => {
$button.click()
expect(stub.getCall(0)).to.be.calledWith('Questions saved!')
cy.get(':nth-child(53)').should('be.visible') // #4
Copy link
Owner

Choose a reason for hiding this comment

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

c'est dangereux ici de check pour index 53. Il serait plutôt recommander de trouver exactement celui que vous venez d'ajouter sans se fier à l'index.

Copy link
Owner

Choose a reason for hiding this comment

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

J'enleve pas de points ici

it('removes a question', () => {
const stub = cy.stub()
cy.on('window:confirm', stub)
cy.get(':nth-child(52) > .delete-button').then(($button) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Même commentaire qu'en haut

Copy link
Owner

Choose a reason for hiding this comment

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

J'enleve pas de points ici

it('show when there is no category selected', () => {
const wrapper = mount(Leaderboards)

expect(wrapper.text()).toContain('Leaderboards') // #1
Copy link
Owner

Choose a reason for hiding this comment

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

Si vous avez accès à un sélecteur plus direct, c'est mieux

}
]
})
vi.stubGlobal('localStorage', {
Copy link
Owner

Choose a reason for hiding this comment

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

super clean!

it('reads local storage', () => {
wrapper = mount(Leaderboards)

expect(wrapper.vm.highScores).toStrictEqual(mockHighScores) // #3
Copy link
Owner

Choose a reason for hiding this comment

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

hmm... il n'y a pas de chances que ce soit autrement puisque vous le mettez directement à la ligne 48. Ici on devrait plutôt faire un mockReturnValue au lieu d'un setItem(). Je ne peux pas compter ce test comme un bon test malheureusement.


await wrapper.vm.updateHighScore()

expect(JSON.parse(localStorage.getItem('highScores'))).toStrictEqual(mockHighScores) // #6
Copy link
Owner

Choose a reason for hiding this comment

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

le getItem() ici n'a pas le choix de valoir mockHighScores, encore une fois à cause que c'est set dans le beforeEach()


describe('initial state', () => {
it('verifies options and categories', () => {
expect(wrapper.findAll('select')[0].findAll('option')[0].text()).toBe('Select a category') // #20
Copy link
Owner

Choose a reason for hiding this comment

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

amélioration possible: faire une variable avec: wrapper.findAll('select')[0].findAll('option')

it('verifies options and categories', () => {
expect(wrapper.findAll('select')[0].findAll('option')[0].text()).toBe('Select a category') // #20
expect(wrapper.findAll('select')[0].findAll('option').length).toEqual(
wrapper.props('categories').length + 1
Copy link
Owner

Choose a reason for hiding this comment

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

amélioration possible: le +1 serait plus clair avec un commentaire

})
})
it('counts down', async () => {
vi.advanceTimersByTime(10000)
Copy link
Owner

Choose a reason for hiding this comment

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

ça pouvait aussi ce faire avec wrapper.tick(10000) si je ne me trompe pas.

}
})
const LeaderboardsComponent = wrapper.findComponent(Leaderboards)
expect(LeaderboardsComponent.exists()).toBeTruthy() // #8
Copy link
Owner

Choose a reason for hiding this comment

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

ça aurait pu être fait dans le même it()

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