Properly remove stale test user dir (#3270)

* BNify

* Add warning

* Add remove_tree and use it for test user dir

src/filesystem.cpp

@@ -1,13 +1,15 @@
 #include "filesystem.h"
 
 // FILE I/O
+#include <stdexcept>
 #include <sys/stat.h>
 #include <cstdlib>
 #include <algorithm>
 #include <cerrno>
 #include <cstdio>
 #include <cstring>
 #include <deque>
+#include <filesystem>
 #include <fstream>
 #include <string>
 #include <vector>
@@ -117,6 +119,19 @@ bool remove_directory( const std::string &path )
 #endif
 }
 
+bool remove_tree( const std::string &path )
+{
+    try {
+        // TODO: C++20 - path constructor should be able to take the string as is
+        auto fs_path = std::filesystem::u8path( path );
+        std::filesystem::remove_all( fs_path );
+    } catch( std::filesystem::filesystem_error &e ) {
+        dbg( DL::Error ) << "remove_tree [" << path << "] failed with \"" << e.what() << "\".";
+        return false;
+    }
+    return true;
+}
+
 const char *cata_files::eol()
 {
 #if defined(_WIN32)

src/filesystem.h

@@ -30,6 +30,12 @@ bool remove_file( const std::string &path );
  * @return true on success, false on failure (e.g. directory is not empty).
  */
 bool remove_directory( const std::string &path );
+/**
+ * Remove a directory and all its children.
+ * @return true on success or if the directory did not exist,
+ *         false on failure to remove (e.g. no permissions, directory is being used).
+ */
+bool remove_tree( const std::string &path );
 /**
  * Rename a file, overwriting the target. Does not overwrite directories.
  * @return true on success, false on failure.

tests/filesystem_test.cpp

@@ -136,13 +136,17 @@ static void filesystem_test_group( int serial, const std::string &s1, const std:
 
     // Can't delete directory with files
     REQUIRE( !remove_directory( dir1 ) );
+    REQUIRE( dir_exist( dir1 ) );
+
+    // Unless we use remove_tree
+    REQUIRE( remove_tree( dir1 ) );
+    REQUIRE( !dir_exist( dir1 ) );
+
+    // Removing non-existent tree is not an error
+    REQUIRE( remove_tree( dir1 ) );
+    REQUIRE( remove_tree( dir2 ) );
 
     // Clean up
-    REQUIRE( remove_file( file1_1 ) );
-    REQUIRE( remove_file( file1_2 ) );
-    REQUIRE( remove_file( file2_1 ) );
-    REQUIRE( remove_directory( dir2 ) );
-    REQUIRE( remove_directory( dir1 ) );
     REQUIRE( remove_directory( base ) );
 }
 

tests/test_main.cpp

@@ -99,7 +99,9 @@ static void init_global_game_state( const std::vector<mod_id> &mods,
                                     option_overrides_t &option_overrides,
                                     const std::string &user_dir )
 {
-    remove_directory( user_dir );
+    if( !remove_tree( user_dir ) ) {
+        assert( !"Unable to remove user_dir directory.  Check permissions." );
+    }
     if( !assure_dir_exist( user_dir ) ) {
         assert( !"Unable to make user_dir directory.  Check permissions." );
     }
@@ -294,9 +296,11 @@ int main( int argc, const char *argv[] )
     // Note: this must not be invoked before all DDA-specific flags are stripped from arg_vec!
     int result = session.applyCommandLine( arg_vec.size(), arg_vec.data() );
     if( result != 0 || session.configData().showHelp ) {
-        cata_printf( "CataclysmDDA specific options:\n" );
+        cata_printf( "Cataclysm: BN specific options:\n" );
         cata_printf( "  --mods=<mod1,mod2,…>         Loads the list of mods before executing tests.\n" );
         cata_printf( "  --user-dir=<dir>             Set user dir (where test world will be created).\n" );
+        cata_printf( "                               Don't use any existing folder you care about,\n" );
+        cata_printf( "                               all contents will be erased!\n" );
         cata_printf( "  -D, --drop-world             Don't save the world on test failure.\n" );
         cata_printf( "  --option_overrides=n:v[,…]   Name-value pairs of game options for tests.\n" );
         cata_printf( "                               (overrides config/options.json values)\n" );