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

[#1501] Create Unit Tests for FeedbackMcqQuestionDetailsTest.java #12638

Merged
merged 17 commits into from
Dec 4, 2023
Merged

[#1501] Create Unit Tests for FeedbackMcqQuestionDetailsTest.java #12638

merged 17 commits into from
Dec 4, 2023

Conversation

mhoualla
Copy link
Contributor

[#1501] Create Unit Tests for Feedback*QuestionDetails classes

Fixes #1501

This PR adds unit tests for the FeedbackMcqQuestionDetails class, focusing on methods related to question details and weights. The tests cover scenarios such as enabling/disabling question dropdowns, validating weights, and handling various combinations of weights and question details.

Current branch coverage of this class is ~80%. Locally, all tests pass successfully.

@cedricongjh
Copy link
Contributor

hi @mhoualla, thank you for your PR, do ensure that the checks on github are passing before we proceed to review it, thank you!

@mhoualla
Copy link
Contributor Author

It appears that the accessibility tests are failing. Could someone please rerun this check? It seems that I don't have the ability to do so. Do these accessibility tests occasionally fail, or have we inadvertently introduced an error into the HTML structure through our unit tests?

@cedricongjh
Copy link
Contributor

It appears that the accessibility tests are failing. Could someone please rerun this check? It seems that I don't have the ability to do so. Do these accessibility tests occasionally fail, or have we inadvertently introduced an error into the HTML structure through our unit tests?

hi @mhoualla as far as I know, the accessibility tests are not known to fail at times, however I also think that it's unlikely that the HTML structure would have changed due to the unit tests, I'll be monitoring and re-running the tests on github, but if you could spare some time, do try to run them locally to ensure that they pass on your branch as well

@mhoualla
Copy link
Contributor Author

Interesting. I commented out my changes, and the accessibility tests are still failing. I want to say that the test failure is completely unrelated to the changes I made, and I would appreciate it if a maintainer is able to take a deeper look into this / the axe test files.

@cedricongjh
Copy link
Contributor

Interesting. I commented out my changes, and the accessibility tests are still failing. I want to say that the test failure is completely unrelated to the changes I made, and I would appreciate it if a maintainer is able to take a deeper look into this / the axe test files.

hi @mhoualla i've ran the accessibility tests on my end as well and they are failing, the team will take a deeper look into this issue, in the meantime, we'll review the current test code

@cedricongjh cedricongjh self-requested a review November 20, 2023 15:52
@cedricongjh cedricongjh self-assigned this Nov 20, 2023
@cedricongjh cedricongjh added the s.ToReview The PR is waiting for review(s) label Nov 20, 2023
cedricongjh
cedricongjh reviewed Nov 20, 2023
View reviewed changes
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

overall LGTM! just one small comment

src/test/java/teammates/common/datatransfer/questions/FeedbackMcqQuestionDetailsTest.java Outdated Show resolved Hide resolved
@cedricongjh cedricongjh added s.FinalReview The PR is ready for final review and removed s.ToReview The PR is waiting for review(s) labels Nov 30, 2023
weiquu
weiquu approved these changes Dec 4, 2023
View reviewed changes
Copy link
Contributor

@weiquu weiquu left a comment

Choose a reason for hiding this comment

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

LGTM

@weiquu weiquu merged commit 8634a78 into TEAMMATES:master Dec 4, 2023
9 checks passed
@mhoualla mhoualla deleted the 1501-add-tests-feedback-mcq-question-details branch December 4, 2023 15:37
@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging c.Task Other non-user-facing works, e.g. refactoring, adding tests and removed s.FinalReview The PR is ready for final review labels Jan 21, 2024
@wkurniawan07 wkurniawan07 added this to the V8.30.0 milestone Jan 21, 2024
cedricongjh pushed a commit to cedricongjh/teammates that referenced this pull request Feb 20, 2024
@mhoualla @cedricongjh
[TEAMMATES#1501] Create Unit Tests for FeedbackMcqQuestionDetailsTest…
5001d8e 
….java (TEAMMATES#12638)

* added tests

* linting and whitespace

* fix failing unit test

* cleaning

* only include default constructor calls

* case

* localize accessibility test faults

* axe testing??

* revert

* remove comments

* update with Arrays.asList

* move assert participant type NONE to constructor

* added unit test for generateOptionsFor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weiquu weiquu weiquu approved these changes

@cedricongjh cedricongjh cedricongjh approved these changes

@javier-garcia-munoz javier-garcia-munoz Awaiting requested review from javier-garcia-munoz

Assignees

@cedricongjh cedricongjh

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.30.0
Development

Successfully merging this pull request may close these issues.

Create Unit Tests for Feedback*QuestionDetails classes
5 participants
@mhoualla @cedricongjh @weiquu @javier-garcia-munoz @wkurniawan07