[
https://issues.apache.org/jira/browse/KUDU-3755?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Marton Greber updated KUDU-3755:
--------------------------------
Description:
The C++ test scaffolding preserves the cluster data directory when a test
fails, controlled by --test_leave_files=on_failure (the default). See
src/kudu/util/test_util.cc:
{code:java}
KuduTest::~KuduTest() {
// Reset the flags first to prevent them from affecting test directory
cleanup.
flag_saver_.reset(); // Clean up the test directory in the destructor
instead of a TearDown
// method. This is better because it ensures that the child-class
// dtor runs first -- so, if the child class is using a minicluster, etc,
// we will shut that down before we remove files underneath.
if (FLAGS_test_leave_files == "always") {
LOG(INFO) << "-----------------------------------------------";
LOG(INFO) << "--test_leave_files specified, leaving files in " << test_dir_;
} else if (FLAGS_test_leave_files == "on_failure" && HasFailure()) {
LOG(INFO) << "-----------------------------------------------";
LOG(INFO) << "Had failures, leaving test files at " << test_dir_;
} else {
VLOG(1) << "Cleaning up temporary test files...";
WARN_NOT_OK(env_->DeleteRecursively(test_dir_),
"Couldn't remove test files");
}
}{code}
The Python and Java test scaffolding do not follow this convention. Both
delegate cluster data cleanup entirely to the kudu test mini_cluster control
shell, which unconditionally deletes the cluster root on exit regardless of
test outcome (src/kudu/tools/tool_action_test.cc):
{code:java}
// Normal exit, clean up cluster root.
if (cluster) {
cluster->Shutdown();
WARN_NOT_OK(Env::Default()->DeleteRecursively(cluster->cluster_root()),
"Could not delete cluster root");
}
return Status::OK(); {code}
Java explicitly documents this delegation
(java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java):
{code:java}
// If a cluster root was not set, create a unique temp directory to use.
// The mini cluster will clean this directory up on exit. try {
File tempRoot = TempDirUtils.makeTempDirectory("mini-kudu-cluster",
TempDirUtils.DeleteOnExit.NO_DELETE_ON_EXIT); {code}
What needs to be done:
# Add a leave_files boolean field to CreateClusterRequestPB in
src/kudu/tools/tool.proto so callers can opt out of the unconditional cleanup
in the control shell
# Implement the opt-out in tool_action_test.cc
# Python (python/kudu/tests/common.py): detect test failure (Python 3:
self._outcome.errors; Python 2: result.failures), pass leaveFiles: true to the
control shell, and manage directory lifetime based on a KUDU_TEST_LEAVE_FILES
env var (on_failure/always/never) mirroring the C++ flag
# Java (KuduTestHarness.java / MiniKuduCluster.java): same, using JUnit 4's
TestWatcher.failed()/succeeded() callbacks to detect outcome
was:
The C++ test scaffolding preserves the cluster data directory when a test
fails, controlled by --test_leave_files=on_failure (the default). See
src/kudu/util/test_util.cc:
{code:java}
KuduTest::~KuduTest() {
// Reset the flags first to prevent them from affecting test directory
cleanup.
flag_saver_.reset(); // Clean up the test directory in the destructor
instead of a TearDown
// method. This is better because it ensures that the child-class
// dtor runs first -- so, if the child class is using a minicluster, etc,
// we will shut that down before we remove files underneath.
if (FLAGS_test_leave_files == "always") {
LOG(INFO) << "-----------------------------------------------";
LOG(INFO) << "--test_leave_files specified, leaving files in " << test_dir_;
} else if (FLAGS_test_leave_files == "on_failure" && HasFailure()) {
LOG(INFO) << "-----------------------------------------------";
LOG(INFO) << "Had failures, leaving test files at " << test_dir_;
} else {
VLOG(1) << "Cleaning up temporary test files...";
WARN_NOT_OK(env_->DeleteRecursively(test_dir_),
"Couldn't remove test files");
}
}{code}
The Python and Java test scaffolding do not follow this convention. Both
delegate cluster data cleanup entirely to the kudu test mini_cluster control
shell, which unconditionally deletes the cluster root on exit regardless of
test outcome (src/kudu/tools/tool_action_test.cc):
{code:java}
// Normal exit, clean up cluster root.
if (cluster) {
cluster->Shutdown();
WARN_NOT_OK(Env::Default()->DeleteRecursively(cluster->cluster_root()),
"Could not delete cluster root");
}
return Status::OK(); {code}
Java explicitly documents this delegation
(java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java):
{code:java}
// If a cluster root was not set, create a unique temp directory to use.// The
mini cluster will clean this directory up on exit.File tempRoot =
TempDirUtils.makeTempDirectory("mini-kudu-cluster",
TempDirUtils.DeleteOnExit.NO_DELETE_ON_EXIT);
{code}
What needs to be done:
# Add a leave_files boolean field to CreateClusterRequestPB in
src/kudu/tools/tool.proto so callers can opt out of the unconditional cleanup
in the control shell
# Implement the opt-out in tool_action_test.cc
# Python (python/kudu/tests/common.py): detect test failure (Python 3:
self._outcome.errors; Python 2: result.failures), pass leaveFiles: true to the
control shell, and manage directory lifetime based on a KUDU_TEST_LEAVE_FILES
env var (on_failure/always/never) mirroring the C++ flag
# Java (KuduTestHarness.java / MiniKuduCluster.java): same, using JUnit 4's
TestWatcher.failed()/succeeded() callbacks to detect outcome
> Python/Java test scaffolding: preserve cluster data on test failure
> -------------------------------------------------------------------
>
> Key: KUDU-3755
> URL: https://issues.apache.org/jira/browse/KUDU-3755
> Project: Kudu
> Issue Type: Bug
> Reporter: Marton Greber
> Priority: Minor
>
> The C++ test scaffolding preserves the cluster data directory when a test
> fails, controlled by --test_leave_files=on_failure (the default). See
> src/kudu/util/test_util.cc:
> {code:java}
> KuduTest::~KuduTest() {
> // Reset the flags first to prevent them from affecting test directory
> cleanup.
> flag_saver_.reset(); // Clean up the test directory in the destructor
> instead of a TearDown
> // method. This is better because it ensures that the child-class
> // dtor runs first -- so, if the child class is using a minicluster, etc,
> // we will shut that down before we remove files underneath.
> if (FLAGS_test_leave_files == "always") {
> LOG(INFO) << "-----------------------------------------------";
> LOG(INFO) << "--test_leave_files specified, leaving files in " <<
> test_dir_;
> } else if (FLAGS_test_leave_files == "on_failure" && HasFailure()) {
> LOG(INFO) << "-----------------------------------------------";
> LOG(INFO) << "Had failures, leaving test files at " << test_dir_;
> } else {
> VLOG(1) << "Cleaning up temporary test files...";
> WARN_NOT_OK(env_->DeleteRecursively(test_dir_),
> "Couldn't remove test files");
> }
> }{code}
> The Python and Java test scaffolding do not follow this convention. Both
> delegate cluster data cleanup entirely to the kudu test mini_cluster control
> shell, which unconditionally deletes the cluster root on exit regardless of
> test outcome (src/kudu/tools/tool_action_test.cc):
> {code:java}
> // Normal exit, clean up cluster root.
> if (cluster) {
> cluster->Shutdown();
> WARN_NOT_OK(Env::Default()->DeleteRecursively(cluster->cluster_root()),
> "Could not delete cluster root");
> }
> return Status::OK(); {code}
> Java explicitly documents this delegation
> (java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java):
> {code:java}
> // If a cluster root was not set, create a unique temp directory to
> use.
> // The mini cluster will clean this directory up on exit. try {
> File tempRoot = TempDirUtils.makeTempDirectory("mini-kudu-cluster",
> TempDirUtils.DeleteOnExit.NO_DELETE_ON_EXIT); {code}
> What needs to be done:
> # Add a leave_files boolean field to CreateClusterRequestPB in
> src/kudu/tools/tool.proto so callers can opt out of the unconditional cleanup
> in the control shell
> # Implement the opt-out in tool_action_test.cc
> # Python (python/kudu/tests/common.py): detect test failure (Python 3:
> self._outcome.errors; Python 2: result.failures), pass leaveFiles: true to
> the control shell, and manage directory lifetime based on a
> KUDU_TEST_LEAVE_FILES env var (on_failure/always/never) mirroring the C++ flag
> # Java (KuduTestHarness.java / MiniKuduCluster.java): same, using JUnit 4's
> TestWatcher.failed()/succeeded() callbacks to detect outcome
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
