[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

I like the fact that you've removed the extra test that you've added.
However, I think that modifying the tests as you've done is more work than is needed.
I *suspect* that all you need to do is to move a couple of test cases from the "calls which should fail" to the "calls that should succeed" list (and adjust for the fact that the calls return false even though they succeed)

For `remove`, that would be `path("dne")` and `path("")` .  Just FYI - I believe that 'dne' is Eric's shorthand for "does not exist".

Something like this:

  TEST_CASE(basic_remove_test)
  {
      scoped_test_env env;
      const path dne = env.make_env_path("dne");
      const path link = env.create_symlink(dne, "link");
      const path nested_link = env.make_env_path("nested_link");
      create_symlink(link, nested_link);
      const path testCases1[] = {
          env.create_file("file", 42),
          env.create_dir("empty_dir"),
          nested_link,
          link
      };
      for (auto& p : testCases1) {
          std::error_code ec = std::make_error_code(std::errc::address_in_use);
          TEST_CHECK(remove(p, ec));
          TEST_CHECK(!ec);
          TEST_CHECK(!exists(symlink_status(p)));
      }
  
  //  This is https://bugs.llvm.org/show_bug.cgi?id=35780
      const path testCases2[] = {
          env.make_env_path("dne"),
          ""
      };
      for (auto& p : testCases2) {
          std::error_code ec = std::make_error_code(std::errc::address_in_use);
          TEST_CHECK(!remove(p, ec));
          TEST_CHECK(!ec);
          TEST_CHECK(!exists(symlink_status(p)));
      }
  }


https://reviews.llvm.org/D41830