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.

Sign up for GitHub

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

Jump to bottom

[#10920] Add tests for calculating statistics #11291

Merged
merged 16 commits into from
Aug 23, 2021
Merged

[#10920] Add tests for calculating statistics #11291

merged 16 commits into from
Aug 23, 2021

Conversation

Jellybeano
Copy link
Contributor

@Jellybeano Jellybeano commented Jul 19, 2021 •
edited
Loading

Part of #10920

Outline of Solution

Add tests for the calculation components of the various questions

Tests completed:

  • mcq-question-statistics
  • msq-question-statistics
  • num-scale-question-statistics
  • rank-options-question-statistics
  • rubric-question-statistics

@Jellybeano Jellybeano marked this pull request as draft July 19, 2021 07:59
@Jellybeano Jellybeano marked this pull request as ready for review July 21, 2021 16:35
@rrtheonlyone rrtheonlyone added the s.ToReview The PR is waiting for review(s) label Jul 25, 2021
daongochieu2810
daongochieu2810 requested changes Jul 25, 2021
View reviewed changes
Copy link
Contributor

@daongochieu2810 daongochieu2810 left a comment

Choose a reason for hiding this comment

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

The tests look good! I left some comments regarding test data

.../app/components/question-types/question-statistics/mcq-question-statistics.component.spec.ts Outdated Show resolved Hide resolved
.../app/components/question-types/question-statistics/msq-question-statistics.component.spec.ts Outdated Show resolved Hide resolved
...omponents/question-types/question-statistics/num-scale-question-statistics.component.spec.ts Outdated Show resolved Hide resolved
@daongochieu2810
Copy link
Contributor

@Jellybeano please click Resolve conversation once ur done resolving it, thanks!

@madanalogy madanalogy added s.Ongoing The PR is being worked on by the author(s) and removed s.ToReview The PR is waiting for review(s) labels Aug 13, 2021
@hhdqirui
Copy link
Member

@daongochieu2810 Hi. For your kind follow up.

@daongochieu2810 daongochieu2810 added s.FinalReview The PR is ready for final review and removed s.Ongoing The PR is being worked on by the author(s) labels Aug 16, 2021
rrtheonlyone
rrtheonlyone requested changes Aug 21, 2021
View reviewed changes
Copy link
Contributor

@rrtheonlyone rrtheonlyone left a comment

Choose a reason for hiding this comment

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

Nice one @Jellybeano

What about these cases:

  • no assigned weights (mcq/msq/rubric)
  • checking that perRecipientResponses is expected (mcq/msq)
  • checking that perRecipientStatsMap is expected (rubric)

@rrtheonlyone rrtheonlyone removed the s.FinalReview The PR is ready for final review label Aug 21, 2021
@rrtheonlyone rrtheonlyone added the s.Ongoing The PR is being worked on by the author(s) label Aug 21, 2021
@Jellybeano
Copy link
Contributor Author

Jellybeano commented Aug 23, 2021 •
edited
Loading

completed adding more test cases as suggested!

rrtheonlyone
rrtheonlyone approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@rrtheonlyone rrtheonlyone left a comment

Choose a reason for hiding this comment

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

LGTM

@rrtheonlyone rrtheonlyone added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.Ongoing The PR is being worked on by the author(s) labels Aug 23, 2021
@rrtheonlyone rrtheonlyone merged commit f9c3709 into TEAMMATES:master Aug 23, 2021
@wkurniawan07 wkurniawan07 added the c.Task Other non-user-facing works, e.g. refactoring, adding tests label Aug 23, 2021
@wkurniawan07 wkurniawan07 added this to the V8.1.0 milestone Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrtheonlyone rrtheonlyone rrtheonlyone approved these changes

@daongochieu2810 daongochieu2810 daongochieu2810 approved these changes

@halfwhole halfwhole Awaiting requested review from halfwhole

Assignees

@rrtheonlyone rrtheonlyone

@daongochieu2810 daongochieu2810

Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V8.1.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Jellybeano @daongochieu2810 @hhdqirui @rrtheonlyone @wkurniawan07 @madanalogy