snuyanzin commented on code in PR #39:
URL:
https://github.com/apache/flink-connector-hive/pull/39#discussion_r3240278525
##########
flink-connector-hive/src/test/java/org/apache/flink/connectors/hive/FlinkEmbeddedHiveRunnerExtension.java:
##########
@@ -64,91 +55,54 @@
import static org.reflections.ReflectionUtils.withAnnotation;
/**
- * JUnit 4 runner that runs hive sql on a HiveServer residing in this JVM. No
external dependencies
- * needed. Inspired by StandaloneHiveRunner.java (almost copied).
+ * JUnit 5 extension that runs hive sql on a HiveServer residing in this JVM.
No external
+ * dependencies needed.
*/
-public class FlinkEmbeddedHiveRunner extends BlockJUnit4ClassRunner {
+class FlinkEmbeddedHiveRunnerExtension implements BeforeAllCallback,
AfterAllCallback {
+
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(FlinkEmbeddedHiveRunnerExtension.class);
- private static final Logger LOGGER =
LoggerFactory.getLogger(FlinkEmbeddedHiveRunner.class);
private HiveShellContainer container;
+ private TemporaryFolder temporaryFolder;
private final HiveRunnerConfig config = new HiveRunnerConfig();
- protected HiveServerContext context;
-
- public FlinkEmbeddedHiveRunner(Class<?> clazz) throws InitializationError {
- super(clazz);
- }
@Override
- protected List<TestRule> classRules() {
- // need to load hive runner config before the context is inited
- loadAnnotatesHiveRunnerConfig(getTestClass().getJavaClass());
- final TemporaryFolder temporaryFolder = new TemporaryFolder();
- context = new FlinkEmbeddedHiveServerContext(temporaryFolder, config);
- List<TestRule> rules = super.classRules();
- ExternalResource hiveShell =
- new ExternalResource() {
- @Override
- protected void before() throws Throwable {
- container =
-
createHiveServerContainer(getTestClass().getJavaClass(), context);
- }
-
- @Override
- protected void after() {
- tearDown();
- }
- };
- rules.add(hiveShell);
- rules.add(temporaryFolder);
- return rules;
- }
+ public void beforeAll(ExtensionContext context) throws Exception {
+ Class<?> testClass = context.getRequiredTestClass();
- @Override
- protected void runChild(final FrameworkMethod method, RunNotifier
notifier) {
- Description description = describeChild(method);
- if (method.getAnnotation(Ignore.class) != null) {
- notifier.fireTestIgnored(description);
- } else {
- EachTestNotifier eachNotifier = new EachTestNotifier(notifier,
description);
- eachNotifier.fireTestStarted();
- try {
- runTestMethod(method, eachNotifier);
- } finally {
- eachNotifier.fireTestFinished();
- }
- }
- }
+ // need to load hive runner config before the context is inited
+ loadAnnotatedHiveRunnerConfig(testClass);
- /** Runs a {@link Statement} that represents a leaf (aka atomic) test. */
- private void runTestMethod(FrameworkMethod method, EachTestNotifier
notifier) {
- Statement statement = methodBlock(method);
+ temporaryFolder = new TemporaryFolder();
+ temporaryFolder.create();
- try {
- statement.evaluate();
- } catch (AssumptionViolatedException e) {
- notifier.addFailedAssumption(e);
- } catch (Throwable e) {
- notifier.addFailure(e);
- }
+ FlinkEmbeddedHiveServerContext hiveContext =
+ new FlinkEmbeddedHiveServerContext(temporaryFolder, config);
+ container = createHiveServerContainer(testClass, hiveContext);
}
- private void tearDown() {
+ @Override
+ public void afterAll(ExtensionContext context) {
if (container != null) {
- LOGGER.info("Tearing down {}", getName());
+ LOGGER.info("Tearing down {}", context.getDisplayName());
try {
container.tearDown();
} catch (Throwable e) {
LOGGER.warn("Tear down failed: " + e.getMessage(), e);
}
}
+ if (temporaryFolder != null) {
+ temporaryFolder.delete();
+ }
Review Comment:
does it mean in case of exception nobody will drop the folder?
why can't we use @TempDir?
--
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]
