Disable TestValidateIncompatibleCentralizedDatasourceSchemaConfig #16627
Conversation
There was a problem hiding this comment.
@findingrish do you have an idea why this test started causing CI failures only recently? I see that this test was introduced in #15817, which was merged more than a month ago.
| @@ -51,6 +52,9 @@ public TestValidateIncompatibleCentralizedDatasourceSchemaConfig(ServerRunnable | |||
| this.runnable = runnable; | |||
| } | |||
|
|
|||
| // It seems that setting the system properties is causing MainTest to fail. | |||
| // Ignoring this test until there is a better way to set the properties. | |||
| @Ignore | |||
| @Test(expected = RuntimeException.class) | |||
There was a problem hiding this comment.
I think this line is the problem. Looking at this test, an exception is expected to be thrown when the runnable gets the system properties in L68-69.
After an exception is thrown, it doesn't clear the properties set from L71-73 causing the incompatible system properties to be "leaked".
A couple of points regarding fixing this test:
- Instead of setting system-wide properties that can affect other tests broadly, you can bind the supplied properties to the Guice injector instance. For example, please see SqlModuleTest.java. This way, the properties would only be scoped to this test injector.
- One of the problems with expected exceptions
@Test(expected = Foo.class)is that it doesn't tell us which line of code is expected to throw the exception. We should useAssert.assertThrows()with a narrow code block around lines 68-69.
There was a problem hiding this comment.
Okay, I see that the test order plays a role here and the order is non-deterministic AFAIK. So maybe it's just a coincidence that this started happening more recently? 🤔
Regardless, ignoring the test looks fine to unblock the CI pipeline. I restarted the two failed cds* ITs, but they have been failing on other PRs too, so we can ignore them for now.
It seems that the system properties being set in this test is causing
org.apache.druid.cli.MainTestto fail, ex: https://github.com/apache/druid/actions/runs/9561791589/job/26357426307?pr=16620.Disabling this test for now, will find another way to set configs.