Skip to content

Conversation

@PranavD45
Copy link

Fixes #13304

Outline of Solution
Refactored tests GetFeedbackResponseCommentActionTest.java (51) and GetFeedbackSessionActionTest.java (53). I did ask to refactor for tests 51 through to 53 but I wasn't able to figure out a solution for GetFeedbackResponsesActionTest.java (52) in a confident and timely manner.

My solution for test 51 simply just cleans up the setup() method and implements a helper function, generateComment() to reduce code bloat and improve readability.

My solution for test 53 moves the login initialization and logout cleanup into the startup() and the (new) tearDown() methods instead of having them called independently for each test. I also implemented a helper function, configureFeedbackSession(), for the repetitive feedback configuration chains in every test.

Note that files other than those specified for refactoring were altered to help me with compilation and so they're not a part of my solutions.

@github-actions
Copy link

Hi @PranavD45, 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.

@PranavD45 PranavD45 changed the title Refactoring for Unit Test Access Controls #13304 - Tests 51 and 53 [#13304] Refactoring for Unit Test Access Controls - Tests 51 and 53 Oct 26, 2025
Updated configuration comments for clarity and specificity.
Updated configuration values for GitHub Actions testing.
@PranavD45
Copy link
Author

Not sure why some of these tests are failing. Is it my implementation or something to do with my project config? I tried reverting some of my files but that doesn't seem to be the issue.

@InfinityTwo
Copy link
Contributor

@PranavD45 thanks for your contributions. On initial looks, I have some comments for you which could help you move forward :)

  • In your PR description, use Part of instead of Fixes so that we do not close the original issue
  • Do NOT commit the files in src/main/resources that you've edited, please
  • The errors for GitHub Action should have only failed for E2E tests, as that is a separate issue, which should be merged soon. Cross-check with other PRs that are also helping to refactor to see what should and should not fail

If you plan to continue the refactor, please assist us by implementing the first two changes. Thanks and happy coding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactoring for Unit Test Access Controls

2 participants