Some missed cases in comma separation

src/cmdstan/command.hpp

@@ -277,8 +277,14 @@ int command(int argc, const char *argv[]) {
       = get_vec_var_context(init, num_chains, id);
 
   if (get_arg_val<bool_argument>(parser, "output", "save_cmdstan_config")) {
+    auto base_file = output_file;
+    // when there are commas, take first file
+    auto comma_pos = base_file.find(',');
+    if (comma_pos != std::string::npos) {
+      base_file = base_file.substr(0, comma_pos);
+    }
     auto config_filename
-        = file::get_basename_suffix(output_file).first + "_config.json";
+        = file::get_basename_suffix(base_file).first + "_config.json";
     auto ofs_args = file::safe_create(config_filename, sig_figs);
     stan::callbacks::json_writer<std::ostream> json_args(std::move(ofs_args));
     write_config(json_args, parser, model);

src/cmdstan/file.hpp

@@ -105,6 +105,13 @@ std::pair<std::string, std::string> get_basename_suffix(
   return {base, suffix};
 }
 
+std::vector<std::string> split_on_comma(const std::string &input) {
+  std::vector<std::string> result;
+  boost::algorithm::split(result, input, boost::is_any_of(","),
+                          boost::token_compress_on);
+  return result;
+}
+
 /**
  * Check if two file paths are the same file.
  * @note This function only handles very basic access patterns.
@@ -117,6 +124,27 @@ std::pair<std::string, std::string> get_basename_suffix(
  */
 bool check_approx_same_file(const std::string &path1,
                             const std::string &path2) {
+  // if path1 has a comma in it, check all of them
+  // if any match, return true
+  if (path1.find(',') != std::string::npos) {
+    for (const auto &name : split_on_comma(path1)) {
+      if (check_approx_same_file(name, path2)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  // same for path2
+  if (path2.find(',') != std::string::npos) {
+    for (const auto &name : split_on_comma(path2)) {
+      if (check_approx_same_file(path1, name)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   const auto path1_size = path1.size();
   const auto path2_size = path2.size();
   if (path1.empty() || path2.empty()) {
@@ -163,6 +191,14 @@ bool check_approx_same_file(const std::string &path1,
  * @param fname candidate output filename
  */
 void validate_output_filename(const std::string &fname) {
+  // if a , is present, check all values
+  if (fname.find(',') != std::string::npos) {
+    for (const auto &name : split_on_comma(fname)) {
+      validate_output_filename(name);
+    }
+    return;
+  }
+
   std::string sep = std::string(1, cmdstan::file::PATH_SEPARATOR);
   if (!fname.empty()
       && (fname[fname.size() - 1] == PATH_SEPARATOR
@@ -195,9 +231,7 @@ std::vector<std::string> make_filenames(const std::string &filename,
 
   // if a ',' is present, we assume the user fully specified the names
   if (filename.find(',') != std::string::npos) {
-    std::vector<std::string> filenames;
-    boost::algorithm::split(filenames, filename, boost::is_any_of(","),
-                            boost::token_compress_on);
+    std::vector<std::string> filenames = split_on_comma(filename);
     if (filenames.size() != num_chains) {
       std::stringstream msg;
       msg << "Number of filenames does not match number of chains: got "

src/test/interface/config_json_test.cpp

@@ -10,34 +10,67 @@ using cmdstan::test::run_command_output;
 class CmdStan : public testing::Test {
  public:
   void SetUp() {
-    multi_normal_model = {"src", "test", "test-models", "multi_normal_model"};
-    arg_output = {"test", "output"};
-    output_csv = {"test", "output.csv"};
-    output_json = {"test", "output_config.json"};
+    multi_normal_model = convert_model_path(
+        std::vector{"src", "test", "test-models", "multi_normal_model"});
+    arg_output = convert_model_path(std::vector{"test", "output"});
+
+    output_csv = convert_model_path(std::vector{"test", "output.csv"});
+    output_json = convert_model_path(std::vector{"test", "output_config.json"});
+
+    output_csv_multi
+        = convert_model_path(std::vector{"test", "output_multi.csv"});
+    output_json_multi
+        = convert_model_path(std::vector{"test", "output_multi_config.json"});
   }
 
   void TearDown() {
-    std::remove(convert_model_path(output_csv).c_str());
-    std::remove(convert_model_path(output_json).c_str());
+    std::remove(output_csv.c_str());
+    std::remove(output_json.c_str());
+    std::remove(output_csv_multi.c_str());
+    std::remove(output_json_multi.c_str());
   }
 
-  std::vector<std::string> multi_normal_model;
-  std::vector<std::string> arg_output;
-  std::vector<std::string> output_csv;
-  std::vector<std::string> output_json;
+  std::string multi_normal_model;
+  std::string arg_output;
+  std::string output_csv;
+  std::string output_json;
+
+  std::string output_csv_multi;
+  std::string output_json_multi;
 };
 
 TEST_F(CmdStan, config_json_output_valid) {
   std::stringstream ss;
-  ss << convert_model_path(multi_normal_model)
-     << " sample output file=" << convert_model_path(arg_output)
+  ss << multi_normal_model << " sample output file=" << arg_output
      << " save_cmdstan_config=1";
   run_command_output out = run_command(ss.str());
   ASSERT_FALSE(out.hasError) << out.output;
-  ASSERT_TRUE(file_exists(convert_model_path(output_csv)));
-  ASSERT_TRUE(file_exists(convert_model_path(output_json)));
+  ASSERT_TRUE(file_exists(output_csv));
+  ASSERT_TRUE(file_exists(output_json));
+
+  std::fstream json_in(output_json);
+  std::stringstream result_json_sstream;
+  result_json_sstream << json_in.rdbuf();
+  json_in.close();
+  std::string json = result_json_sstream.str();
+
+  ASSERT_FALSE(json.empty());
+  ASSERT_TRUE(is_valid_JSON(json));
+}
+
+TEST_F(CmdStan, config_json_output_valid_multi) {
+  std::stringstream ss;
+  ss << multi_normal_model
+     << " sample num_chains=2 output file=" << output_csv_multi << ","
+     << output_csv << " save_cmdstan_config=true";
+  run_command_output out = run_command(ss.str());
+  ASSERT_FALSE(out.hasError) << out.output;
+  ASSERT_TRUE(file_exists(output_csv_multi));
+  ASSERT_TRUE(file_exists(output_csv));
+  ASSERT_TRUE(file_exists(output_json_multi));
+  ASSERT_FALSE(file_exists(output_json));
 
-  std::fstream json_in(convert_model_path(output_json));
+  std::fstream json_in(output_json_multi);
   std::stringstream result_json_sstream;
   result_json_sstream << json_in.rdbuf();
   json_in.close();
@@ -49,10 +82,9 @@ TEST_F(CmdStan, config_json_output_valid) {
 
 TEST_F(CmdStan, config_json_output_not_requested) {
   std::stringstream ss;
-  ss << convert_model_path(multi_normal_model)
-     << " sample output file=" << convert_model_path(arg_output);
+  ss << multi_normal_model << " sample output file=" << arg_output;
   run_command_output out = run_command(ss.str());
   ASSERT_FALSE(out.hasError);
-  ASSERT_TRUE(file_exists(convert_model_path(output_csv)));
-  ASSERT_FALSE(file_exists(convert_model_path(output_json)));
+  ASSERT_TRUE(file_exists(output_csv));
+  ASSERT_FALSE(file_exists(output_json));
 }

src/test/interface/file_test.cpp

@@ -75,6 +75,12 @@ TEST(CommandHelper, validate_output_filename) {
 
   std::string fp4 = "foo.bar" + sep + ".";
   EXPECT_THROW(validate_output_filename(fp4), std::invalid_argument);
+
+  std::string fp5 = "foo.bar,baz.gak";
+  EXPECT_NO_THROW(validate_output_filename(fp5));
+
+  std::string fp6 = "foo.bar,foo.bar" + sep + ".";
+  EXPECT_THROW(validate_output_filename(fp6), std::invalid_argument);
 }
 
 TEST(CommandHelper, make_filenames) {
@@ -253,4 +259,8 @@ TEST(CommandHelper, check_same_file_test) {
   EXPECT_FALSE(cmdstan::file::check_approx_same_file(path, dot_path_bad));
   std::string dot_path_good = "a/b/file.txt";
   EXPECT_TRUE(cmdstan::file::check_approx_same_file(path, dot_path_good));
+
+  std::string comma_path = path + "," + path;
+  EXPECT_TRUE(cmdstan::file::check_approx_same_file(path, comma_path));
+  EXPECT_TRUE(cmdstan::file::check_approx_same_file(comma_path, path));
 }