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.

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

[#12588] Improve test code coverage of core components #13000

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

skylar-mo
Copy link

Fixes #12588

Outline of Solution

I added some unit tests to help improve code coverage for "QuestionEditAnswerForm".

@cedricongjh
Copy link
Contributor

hi @skylar-mo, thank you for your PR, do fix the failing tests before we proceed to review it

also do refer to the guidelines here: https://teammates.github.io/teammates/unit-testing.html for writing unit tests, at a glance it appears that the method name is missing from the test descriptor strings, do add that in, thank you! also there is no need to add the test case in a new describe block, do remove that as well

@cedricongjh cedricongjh self-requested a review April 9, 2024 12:44
@cedricongjh cedricongjh added the s.Ongoing The PR is being worked on by the author(s) label Apr 9, 2024
Copy link
Contributor

@Andy-W-Developer Andy-W-Developer left a comment

Choose a reason for hiding this comment

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

your tests are outside of the describe block
move them below the "should create:" test

expect(component).toBeTruthy();
});
});

it('updateNoneOfTheAbove: should update answers based on isNoneOfTheAboveEnabled', () => {
const component = new MsqQuestionEditAnswerFormComponent();
Copy link
Contributor

Choose a reason for hiding this comment

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

component is declared in beforeEach
this can be deleted


it('updateNoneOfTheAbove: should update answers based on isNoneOfTheAboveEnabled', () => {
const component = new MsqQuestionEditAnswerFormComponent();
component.responseDetails = {
Copy link
Contributor

Choose a reason for hiding this comment

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

the type of responseDetails is
FeedbackMsqResponseDetails extends FeedbackResponseDetails
so the three missing properties and their types are

isOther: boolean
otherFieldContent: string
questionType: FeedbackQuestionType

FeedbackQuestionType is an enum

use createBuilder to create responseDetails so you don't have to enter all four properties every time you create responseDetails, here is an example of createBuilder
https://teammates.github.io/teammates/unit-testing.html#creating-test-data

you will need these imports

import { createBuilder } from '../../../../test-helpers/generic-builder';
import { FeedbackMsqResponseDetails } from '../../../../types/api-output';
import { FeedbackQuestionType } from '../../../../types/api-request';

// Simulate isNoneOfTheAboveEnabled being false
component.updateNoneOfTheAbove();

expect(component.responseDetails.answers).toEqual(['NoneOfTheAbove']);
Copy link
Contributor

Choose a reason for hiding this comment

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

responseDetails has an @Input() decorator so it is set by the parent component, not updateNoneOfTheAbove()
responseDetails.answers is not used for data binding in the html

so instead of testing the value of responseDetails.answers
test the behavior of updateNoneOfTheAbove()

it calls this.triggerResponseDetailsChangeBatch() with values for the properties answers: isOther: otherFieldContent:

use testEventEmission
https://teammates.github.io/teammates/unit-testing.html#testing-event-emission

import testEventEmission from '../../../../test-helpers/test-event-emitter';

you will need to look at the parent class of MsqQuestionEditAnswerFormComponent
which is QuestionEditAnswerFormComponent

it is in the file question-edit-answer-form.ts, the same folder as your test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s.Ongoing The PR is being worked on by the author(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve test code coverage of core components
3 participants