[#12588] Improve test code coverage of QuestionEditDetailsForm #12999
Conversation
|
hi @rushithapenikalapati, 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! |
There was a problem hiding this comment.
some unrelated files were added/removed with your initial commit hash 176da54
you can get them back with git revert --no-commit 176da54461
| @@ -65,7 +66,7 @@ describe('ConstsumOptionsQuestionEditDetailsFormComponent', () => { | |||
| expect(eventSpy).not.toHaveBeenCalled(); | |||
| }); | |||
|
|
|||
| it('should allow number input with less than or equal to 9 digits', () => { | |||
| it('restrictIntegerInputLength: should allow number input with less than or equal to 9 digits when a number is inputted', () => { | |||
There was a problem hiding this comment.
this line is too long (120+ characters)
you can split it like this
'restrictIntegerInputLength: should allow number input with less than or equal to 9 digits '
+ 'when a number is inputted',
with the + on the new line
there are more lines that are too long
GitHub should mark out these linting issues in the Files changed tab
| const initialLength = component.model.constSumOptions.length; | ||
| fixture.detectChanges(); | ||
| expect(component.model.constSumOptions.length).toBe(initialLength + 1); | ||
| expect(component.model.constSumOptions[initialLength]).toBe(''); |
There was a problem hiding this comment.
increaseNumberOfConstsumOptions() is not called
all the functions you are testing emit events
test this behavior instead with testEventEmission
import testEventEmission from '../../../../test-helpers/test-event-emitter';
https://teammates.github.io/teammates/unit-testing.html#testing-event-emission
fixture.detectChanges() is not needed now that you are testing emits
| it('onConstsumOptionDeleted: should successfully delete a constSumOption when more than 2 options exist', () => { | ||
| component.model = { ...component.model, constSumOptions: ['Option 1', 'Option 2', 'Option 3'] }; | ||
| fixture.detectChanges(); | ||
|
|
There was a problem hiding this comment.
linting does not want spaces on empty lines
GitHub will label these lines as having trailing spaces
| expect(component.model.maxPoint).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('detectChanges: should set minPoint to 0 when event is true', () => { |
There was a problem hiding this comment.
accidentally typed detectChanges: instead of resetMinPoint:
Part of #12588
Outline of Solution
I added unit tests for all of the sections highlighted in red for the src/web/app/components/question-types/question-edit-details-form/constsum-options-question-edit-details-form.component.spec.ts file.