[jira] [Updated] (KUDU-3755) Python/Java test scaffolding: preserve cluster data on test failure

["Marton Greber (Jira)" <ji...@apache.org>]

     [ 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)