[PATCH] D86913: Add raw_fd_stream that supports reading/seeking/writing

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/unittests/Support/raw_fd_stream_test.cpp:1
+//===- llvm/unittest/Support/raw_fd_stream_test.cpp - raw_fd_stream tests -===//
+//
----------------
stephan.yichao.zhao wrote:
> jhenderson wrote:
> > A couple of other test cases I'd like to see, depending on how practical it is (I suspect it's not practical, and if so, don't worry).
> > 
> > 1) An unseekable input (i.e. where `supportsSeeking` returns `false`).
> > 2) An unseekable input, but where `EC` was already in an error state (and so the new error wasn't reported).
> Looked into gmock a bit. To mock supportsSeeking, the code under test needs to rewrite to use either virtual method or templete: https://github.com/google/googletest/blob/master/googlemock/docs/cook_book.md#MockingNonVirtualMethods
> 
> Another way to test it is like this. At raw_fd_ostream::raw_fd_ostream, at a place where SupportsSeeking is set, we do
> 
>   if GlobalMockValue is set:
>     SupportsSeeking =  GlobalMockValue
>   else
>     SupportsSeeking = ...
> 
> Here GlobalMockValue could be a global pointer. And the unit test can set it. EC can be tested in the same way.
> 
> But this also needs to change the code under test. Did any other LLVM test code do something similar?
Yeah, I didn't think this through hard enough, and I don't see a great way of doing this in the tests (I don't think rewriting the code for greater testability is necessary). If you can manually locally verify that the code works as desired, then that would be sufficient for my comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86913/new/

https://reviews.llvm.org/D86913