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] Test coverage improvements for InstructorResponsesViewBase #13087

Conversation

justinsegawa
Copy link
Contributor

Part of #12588

I created new units tests for all lines not covered in InstructorResponsesViewBase. Since this is an abstract class I wrote tests in the file per-question-view-responses.component.spec.ts, since PerQuestionViewResponsesComponent extends InstructorResponsesViewBase.

Copy link

Hi @justinsegawa, thank you for your interest in contributing to TEAMMATES!
However, your PR does not appear to follow our contribution guidelines:

  • Title must start with the issue number the PR is fixing in square brackets, e.g. [#<issue-number>]

Please address the above before we proceed to review your PR.

@justinsegawa justinsegawa changed the title Test coverage improvements for InstructorResponsesViewBase [#12588] Test coverage improvements for InstructorResponsesViewBase Apr 24, 2024
@cedricongjh
Copy link
Contributor

hi @justinsegawa, thank you for your PR, do ensure that the frontend tests pass before we proceed to review it, it appears that it's currently failing (see component tests, line 183 of per-question-view-responses.component.spec.ts is causing a failure)

@cedricongjh cedricongjh added the s.Ongoing The PR is being worked on by the author(s) label Apr 26, 2024
@cedricongjh cedricongjh self-requested a review April 26, 2024 02:08
@weiquu weiquu added s.ToReview The PR is waiting for review(s) and removed s.Ongoing The PR is being worked on by the author(s) labels Jun 29, 2024
Copy link
Contributor

@domoberzin domoberzin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR

@domoberzin domoberzin added s.FinalReview The PR is ready for final review and removed s.ToReview The PR is waiting for review(s) labels Jul 11, 2024
@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 8 days). 🐌 😢
Hope someone can get it to move forward again soon...

@domoberzin domoberzin merged commit 6b7912c into TEAMMATES:master Jul 24, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s.FinalReview The PR is ready for final review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants