capistrant opened a new issue #9333: Investigate Calcite Unit Tests to find if there is a better way to wire up Calcite charset/unicode configs in tests. URL: https://github.com/apache/druid/issues/9333 #5451 was implemented to fix #4909 in the past. Basically, Calcite currently reads from System.properties to pick up some configs and when tests spin up Calcite without setting those properties they can cause failures in the Calcite tests that require those properties being setup how we would when the sql module starts up under normal Druid operation. The fix in 5451 requires all new Calcite test class implementors to know to make their class inherit from the base class implemented in the PR. Otherwise, if the new test class without the inheritance starts up calcite without setting the System properties, we will get failed tests because even though the later tests will try to set the properties themselves, they will already have been read by calcite. Is there a more graceful solution than relying on developers to know to add this to their new test classes? I don't think the current solution is necessarily bad in nature, it just causes PRs like #9324 to have to be made to remediate the problem if TravisCI happens to pass but some or all local developers face issues. Potential solutions? * Is there a way to ensure that a specific test gets run first by unit test suite for the sql module? If that is the case we could use a noop test that gets those System properties set properly up front every time. * Is there a way to use something other than System properties to set these configurations that we can leverage and is more desirable than the current pattern?
---------------------------------------------------------------- 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 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
