paul-rogers commented on issue #1977: DRILL-7573: Support htpasswd based authentication URL: https://github.com/apache/drill/pull/1977#issuecomment-584816302 @dobesv, the key failures to check for are any checkstyle errors. If anything else fails, it is probably due to problems with the (new) builds or master; we'll have to check. This PR includes no tests. It would be great to add some. Some example test cases: * htpasswd enabled, but no file path or default file found. * Enabled, and valid file in default location. * Enabled, and valid file in custom location. * File contents change to become invalid. * File contents change and are valid. To test you'll need to first set up the server to enable the web server, which is turned off by default in tests. For testing, we use something called the `ClusterFixture`. Most tests are based off of `ClusterTest` which set up the server once per test file. For your tests, you may want to start the server in each test case, each with a different configuration. `StatusResourcesTest` is a good example. The config you need is probably something like: ``` ClusterFixtureBuilder builder = ClusterFixture.builder(dirTestWatcher). configProperty(ExecConstants.HTTP_ENABLE, true). configProperty(ExecConstants.HTTP_PORT_HUNT, true). configProperty(ExecConstants.HTPASSWD_AUTHENTICATOR_PATH, "whatever"); ``` Next you'll need a way to talk to the REST server. There is a `RestClientFixture` class which looks promising, though I've not used it myself. You will need a place to create and update the file. The `BaseDirTestWatcher` class does this, often provided in tests as `dirTestWatcher`. See `TestDotDrillUtil` for an example of getting a test-specific temp directory and writing a file to that location. You can then use the temp directory as your custom auth file location when configuring the Drillbit. With this, you can test the "happy path": * Enabled, and valid file in custom location. * File contents change and are valid. A bit more work is needed for the other cases. To test this one: * Enabled, and valid file in default location. You can put the htpasswd in `src/test/resources` which will be on the classpath when running tests. The challenge is that the file is static; if we have the file, there is no way to test errors if the file *does not* exist. Sigh. Finally, checking the error cases is a nice-to-have, but is rather tricky. The errors don't kill the server, they only leave a trace in the log file. So, you can use two other tricks. One is `LogFixture` which lets you temporarily turn on logging. You won't need that because your code logs errors at the `error` level which is always enabled. To capture log messages, you can try something like `TestOperatorDump`: create an in-memory log appender. Yes, it would be nice if this capability were part of `LogFixture`; a nice little project when someone has the time. My recommendation is to at least test the "happy path"; add the others only if you feel so inclined.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
