[C++] Implement generic filesystem tests for GCS

[asfimport]

Once the GCS filesystem implementation is functionally complete, generic tests should be enabled for it to catch corner cases. See for example the S3 filesystem tests:

arrow/cpp/src/arrow/filesystem/s3fs_test.cc

Lines 1039 to 1081 in 3e666f6

	class TestS3FSGeneric : public S3TestMixin, public GenericFileSystemTest {
	public:
	void SetUp() override {
	S3TestMixin::SetUp();
	// Set up test bucket
	{
	Aws::S3::Model::CreateBucketRequest req;
	req.SetBucket(ToAwsString("s3fs-test-bucket"));
	ASSERT_OK(OutcomeToStatus(client_->CreateBucket(req)));
	}
	options_.ConfigureAccessKey(minio_.access_key(), minio_.secret_key());
	options_.scheme = "http";
	options_.endpoint_override = minio_.connect_string();
	ASSERT_OK_AND_ASSIGN(s3fs_, S3FileSystem::Make(options_));
	fs_ = std::make_shared<SubTreeFileSystem>("s3fs-test-bucket", s3fs_);
	}
	protected:
	std::shared_ptr<FileSystem> GetEmptyFileSystem() override { return fs_; }
	bool have_implicit_directories() const override { return true; }
	bool allow_write_file_over_dir() const override { return true; }
	bool allow_move_dir() const override { return false; }
	bool allow_append_to_file() const override { return false; }
	bool have_directory_mtimes() const override { return false; }
	bool have_flaky_directory_tree_deletion() const override {
	#ifdef _WIN32
	// Recent Minio versions on Windows may not register deletion of all
	// directories in a tree when doing a bulk delete.
	return true;
	#else
	return false;
	#endif
	}
	bool have_file_metadata() const override { return true; }
	S3Options options_;
	std::shared_ptr<S3FileSystem> s3fs_;
	std::shared_ptr<FileSystem> fs_;
	};
	GENERIC_FS_TEST_FUNCTIONS(TestS3FSGeneric);

Reporter: Antoine Pitrou / @pitrou
Assignee: Carlos O'Ryan / @coryan

Related issues:

• [C++] GcsFileSystem should use metadata to distinguish directory markers (is a parent of)
• [C++] GcsFileSystem *File classes fail to report errors (is a parent of)
• [C++] Some filesystems may allow reading directories (is a parent of)
• [C++] Add filesystem / IO implementation for Google Cloud Storage (is a child of)

PRs and other links:

• GitHub Pull Request #12127

_{Note: This issue was originally created as ARROW-14924. Please see the migration documentation for further details.}

[asfimport]

Antoine Pitrou / @pitrou:
cc @coryan

[asfimport]

Carlos O'Ryan / @coryan:
I am having problems getting these tests to pass. For example, consider this section of the TestCreateDir() test:

arrow/cpp/src/arrow/filesystem/test_util.cc

Lines 223 to 226 in 3e666f6

	// Cannot create a directory as child of a file
	CreateFile(fs, "AB/def", "");
	ASSERT_RAISES(IOError, fs->CreateDir("AB/def/EF/GH", true /* recursive */));
	ASSERT_RAISES(IOError, fs->CreateDir("AB/def/EF", false /* recursive */));

Recall that GCS does not have directories, it has a flat namespace. We have been using files with a trailing / character as sentinels to indicate that a directory exists. Because GCS has a flat namespace an object called foo is distinct from an object called foo/ (or foo/// for that matter). Therefore, it is possible to have a "file" called AB/def and a "directory" called AB/def/ at the same time. The test fails because creating the directory sentinel AB/def/ succeeds, even though the file AB/def already exists.

I think there are several options, but I am not sure which one is more desirable:

1. We could try to use AB/def as the directory sentinel. I think that would make this test pass, but has some downsides: other places in the code would need to determine if an object without a trailing / is really a directory. That we could do using metadata attributes. However, using metadata attributes would fail when using directory hierarchies created by other tools (say the GCS UI at https://console.cloud.google.com). In my opinion, this is a minor downside because any set of files created externally may be missing the directory sentinels altogether.
2. Alternatively, when recursively creating a directory we could verify that each component does not have a "file" with the same name as the directory marker. This seems expensive, it requires checking two different paths, not to mention the race conditions.

Maybe there are other alternatives that I do not see at this moment. At the moment I am inclined to use the first alternative.

[asfimport]

Antoine Pitrou / @pitrou:
I agree the first alternative sounds much better than the second alternative.

There is a third alternative, though, which is to define an additional "deviation" in the generic tests, such as those that already exist:
https://github.com/apache/arrow/blob/master/cpp/src/arrow/filesystem/test_util.h#L159-L178

I'll let you choose which alternative is the most reasonable, you're the expert here :-)

[asfimport]

Antoine Pitrou / @pitrou:
Issue resolved by pull request 12127
#12127