GH-43677: [C++][FlightRPC] Move the FlightTestServer to its own .cc and .h files#43678
GH-43677: [C++][FlightRPC] Move the FlightTestServer to its own .cc and .h files#43678felipecrv merged 3 commits intoapache:mainfrom
Conversation
|
Pushed a commit moving the auth handlers as well. |
lidavidm
left a comment
There was a problem hiding this comment.
I'll just note that these are very much not meant as pedagogical examples
| std::unique_ptr<FlightServerBase> TestFlightServer::Make() { | ||
| return std::make_unique<TestFlightServer>(); | ||
| } |
There was a problem hiding this comment.
Do we need this factory method?
How about using std::make_unique<TestFlightServer>() directly in callers?
There was a problem hiding this comment.
It's used quite a lot and is handy to not have to write the longer make_unique invocation. This is replacing the old free function called ExampleTestServer.
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 69bce8f. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
One way of learning about a codebase is reading the tests. As it is now, it's hard to see the minimal
FlightServerBasesub-class inflight/test_util.cc, so I moved it to its own file.What changes are included in this PR?
FlightTestServertoTestFlightServertest_flight_server.{h,cc}test_auth_handlers.{h,cc}Are these changes tested?
By existing tests.
Are there any user-facing changes?
ExampleTestServeris removed from the testing library in favor ofFlightTestServer::Make.