kfaraz commented on PR #18207: URL: https://github.com/apache/druid/pull/18207#issuecomment-3130797482
> One thing I was thinking about while reviewing is that i believe we are perhaps losing some minor coverage of auth stuff during this transition since I think the base ITs had basic-auth setup, though afaict not much in the way of roles and stuff in most tests (so i think was only authentication that would have been tested except for those that extend AbstractAuthConfigurationTest). I think that is probably fine though as long as we migrate the auth tests over to run on this framework, though they probably don't cast quite a wide of net in terms of APIs being called since those tests are more focused on authorization. I don't think I am suggesting we just bake basic auth into random tests or anything, and really its a bit of a negative of the old frameworks that you have to hunt across several files to determine what configuration is actually active for a given test, but maybe something we should watch out for as we move tests over. Thanks for calling this out, @clintropolis ! I do plan to migrate the auth tests as well to the embedded framework. For now, we will migrate the non-auth tests without basic auth enabled. Once all tests are migrated, we can add test variants which have auth enabled to increase API coverage. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
