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

IHRDS 3159 handle bad request body for /users/groups/query #872

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

lofoyet
Copy link
Contributor

@lofoyet lofoyet commented Oct 6, 2023

Summary

  • handle invalid request body for endpoint "users" / "groups" / "query"
  • added test case (don't expect them to be in the right format)

Reference

I don't expect to write unit test like this, so I am open to any change.

@lofoyet lofoyet changed the title IHRDS 3159 handle bad request body for /users/groups/query' IHRDS 3159 handle bad request body for /users/groups/query Oct 6, 2023
@lofoyet lofoyet requested a review from kailuowang October 6, 2023 16:43
Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

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

Looks great to me. The only nitpick I have is that the test should probably be in the com.iheart.thomas.http4s.abtest package.

@lofoyet
Copy link
Contributor Author

lofoyet commented Oct 9, 2023

com.iheart.thomas.http4s.abtest

good point, done

@kailuowang
Copy link
Contributor

hmm you might have to move the TestUtil into com.iheart.thomas package.

@lofoyet
Copy link
Contributor Author

lofoyet commented Oct 10, 2023

hmm you might have to move the TestUtil into com.iheart.thomas package.

I am not sure why it gave an error. locally the test looks ok.
i can try to move move the TestUtil into com.iheart.thomas package. to get some clarification, TestUtils is will still be in the tests/it/ package instead of the tests/test/ package right?

@lofoyet
Copy link
Contributor Author

lofoyet commented Oct 10, 2023

interesting, when i run that test alone it's ok, when i do sbt validate i got the same error

@kailuowang
Copy link
Contributor

kailuowang commented Oct 10, 2023

I don't know why it passes when you run it alone. But your new test is not in the right folder. It should be in the It folder since it's an integration tests. tests in tests/test folder won't be able to access TestUtils

@lofoyet
Copy link
Contributor Author

lofoyet commented Oct 10, 2023

Yes, you are right, i just realized so. my test depends on mongo, so just like you said, it fits more like an it test maybe not unit test.
I think moving the EndpointSuite to src/it/ is the right way to go. what's your thought?
@kailuowang

@kailuowang
Copy link
Contributor

kailuowang commented Oct 10, 2023 via email

@lofoyet lofoyet merged commit fff1444 into master Oct 11, 2023
@lofoyet lofoyet deleted the IHRDS-3159 branch October 11, 2023 16:24
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.

2 participants