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

[#12048] Migrate tests for GetAccountActionTest #13212

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

[#12048] Migrate tests for GetAccountActionTest #13212

wants to merge 11 commits into from
+93 −1

Conversation

mingyang143
Copy link
Contributor

@mingyang143 mingyang143 commented Feb 3, 2025 •
edited
Loading

Part of #12048

Outline of Solution
Change GetAccountActionTest.java to ensure compatibility with the PostgreSQL database following the database migration.

@mingyang143 mingyang143 force-pushed the branch-migrate-tests-week4 branch from e68354c to 8a00cea Compare February 8, 2025 04:56
DhiraPT
DhiraPT suggested changes Feb 9, 2025
View reviewed changes
Copy link
Contributor

@DhiraPT DhiraPT left a comment

Choose a reason for hiding this comment

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

Overall, the changes look good! The implementation is clear, and the tests cover the main functionality well. Consider adding a few additional tests to improve coverage and refining the test method name for better clarity. Great work so far!

@dishenggg dishenggg added the s.Ongoing The PR is being worked on by the author(s) label Feb 9, 2025
@dishenggg dishenggg mentioned this pull request Feb 11, 2025
Merged
@jasonqiu212 jasonqiu212 removed the s.Ongoing The PR is being worked on by the author(s) label Feb 18, 2025
jasonqiu212
jasonqiu212 approved these changes Feb 18, 2025
View reviewed changes
Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

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

LGTM! good work

@jasonqiu212 jasonqiu212 added the s.FinalReview The PR is ready for final review label Feb 18, 2025
@dishenggg dishenggg added s.Ongoing The PR is being worked on by the author(s) and removed s.FinalReview The PR is ready for final review labels Feb 19, 2025
@jasonqiu212 jasonqiu212 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 Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonqiu212 jasonqiu212 jasonqiu212 approved these changes

@DhiraPT DhiraPT Awaiting requested review from DhiraPT

@dishenggg dishenggg Awaiting requested review from dishenggg

Requested changes must be addressed to merge this pull request.

Assignees

@mingyang143 mingyang143

Labels
s.FinalReview The PR is ready for final review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mingyang143 @dishenggg @jasonqiu212 @DhiraPT