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

[#12048] Migrate FeedbackMsqQuestionE2ETest #12904

Merged
merged 14 commits into from Mar 19, 2024

Conversation

dishenggg
Copy link
Contributor

@dishenggg dishenggg commented Mar 16, 2024

Part of #12048

Outline of Solution

Update E2E test to use Sql entities

Edit:
Update FeedbackMsqQuestion.makeDeepCopy() to use FeedbackMsqQuestionDetails.getDeepCopy()
Update FeedbackQuestionLogic.getFeedbackQuestionsForStudents() to sort the questions by question number

@dishenggg dishenggg self-assigned this Mar 16, 2024
@dishenggg dishenggg marked this pull request as ready for review March 16, 2024 12:10
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.

hey @dishenggg, looks good so far but I think the test needs to be added to src/e2e/resources/testng-e2e-sql.xml for it to run + data should be loaded into a separate json (see tele msg)

@dishenggg dishenggg marked this pull request as draft March 18, 2024 08:09
@dishenggg dishenggg marked this pull request as ready for review March 18, 2024 13:07
@weiquu
Copy link
Contributor

weiquu commented Mar 18, 2024

@dishenggg can take a look and see why it's failing? thanks!

@dishenggg
Copy link
Contributor Author

@weiquu The questions were not sorted properly, fixed it!

@cedricongjh cedricongjh requested a review from weiquu March 18, 2024 20:05
@cedricongjh cedricongjh added the s.FinalReview The PR is ready for final review label Mar 18, 2024
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.

LGTM, thanks! good catches on the bugs

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 4dc0c6d into TEAMMATES:master Mar 19, 2024
11 checks passed
@cedricongjh cedricongjh added this to the V9.0.0-beta.1 milestone Mar 24, 2024
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.

None yet

3 participants