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

feat: add unit tests to SearchServiceTests #423

Conversation

alter-kaker
Copy link

@alter-kaker alter-kaker commented Jun 20, 2024

  • add testSearchPostByUrlReturnsEmptyWhenNoPosts - fails, disabled
  • add testSearchPostByUrlWithCrossPostsFewerPostsThanPageSize - fails, disabled
  • add testSearchPostByUrlWithCrossPostsOnePageWorthOfPostsRequestSecondPage - fails, disabled
  • rename testSearchPostByUrlWithCrossPostsReturnsExactlyOnePageWorthOfResults - passes
  • add testSearchPostByUrlWithCrossPostsPageOneOfTwo - passes
  • add testSearchPostByUrlWithCrossPostsPageTwoOfTwo - passes

closes #407

@alter-kaker alter-kaker requested a review from a team as a code owner June 20, 2024 02:50
add testSearchPostByUrlReturnsEmptyWhenNoPosts - fails, disabled
add testSearchPostByUrlWithCrossPostsFewerPostsThanPageSize - fails, disabled
add testSearchPostByUrlWithCrossPostsOnePageWorthOfPostsRequestSecondPage - fails, disabled
rename testSearchPostByUrlWithCrossPostsReturnsExactlyOnePageWorthOfResults - passes
add testSearchPostByUrlWithCrossPostsPageOneOfTwo - passes
add testSearchPostByUrlWithCrossPostsPageTwoOfTwo - passes
Copy link
Member

@lazyguru lazyguru left a comment

Choose a reason for hiding this comment

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

My only comment here would be that it would probably be better to move some of the repeated blocks of code into either a setup() call or have a separate function you could call to configure the tests where you reuse that same block of code. Otherwise, thanks for the contribution! I'm going to leave the PR unmerged for 1 day to allow @jgrim or @Pdzly to weigh in if they want

@lazyguru
Copy link
Member

Also, @alter-kaker would you be able to configure your git client to sign your commits?

Pdzly
Pdzly approved these changes Jun 24, 2024
@Pdzly Pdzly enabled auto-merge June 24, 2024 06:23
@Pdzly Pdzly disabled auto-merge June 24, 2024 06:23
@Pdzly Pdzly enabled auto-merge June 29, 2024 13:28
@lazyguru
Copy link
Member

@alter-kaker please sign your commits

@lazyguru lazyguru disabled auto-merge July 13, 2024 04:52
@lazyguru
Copy link
Member

I'm closing this PR as there has been no response to my requests for the commits to be signed. When/if you are able to sign your commits, please feel free to reopen

@lazyguru lazyguru closed this Jul 24, 2024
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.

Add more unit test coverage to Search package
3 participants