niyue commented on a change in pull request #11486:
URL: https://github.com/apache/arrow/pull/11486#discussion_r740208514
##########
File path: cpp/src/arrow/filesystem/mockfs.cc
##########
@@ -242,8 +242,32 @@ class MockFSInputStream : public io::BufferReader {
return metadata_;
}
+ Result<int64_t> DoReadAt(int64_t position, int64_t nbytes, void* out)
override {
+ RecordReadIo(position, nbytes);
+ return io::BufferReader::DoReadAt(position, nbytes, out);
+ }
+
+ Result<std::shared_ptr<Buffer>> DoReadAt(int64_t position, int64_t nbytes)
override {
+ RecordReadIo(position, nbytes);
+ return io::BufferReader::DoReadAt(position, nbytes);
+ }
Review comment:
@westonpace I follow your instruction to use the `MockFileSystem` to add
unit tests. To do this, I tried several ways.
1) initially, I tried to call `GetRecordedReads` of `MockFSInputStream` in
unit test to get the recorded IO, but I found this class is not public in the
`mockfs.h` and I cannot use this class directly.
2) After that, I tried to override the `ReadAt` virtual method in
`io::BuffferReader` but I found `io::BufferReader` inherits from
`internal::RandomAccessFileConcurrencyWrapper`, which marks `ReadAt` as
`final`, which prevents me from overriding it.
Currently, I have to change the `protected` methods `DoReadAt` in
`io::BufferReader` to be `virtual` so that it can be overridden for this kind
of unit testing. But I am not entirely sure if this is desirable, please
advise. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
