Re: [PR] fix(cpp): Handle npos in PathToDirectory for S3 URIs without query strings [incubator-graphar]

["Sober7135 (via GitHub)" <gi...@apache.org>]

Sober7135 commented on PR #888:
URL: https://github.com/apache/incubator-graphar/pull/888#issuecomment-3980630846

   > Yes, the C++ test suite was compiled and run locally, and all tests pass successfully.
   
   This is not quite accurate, since no test case was added for this bug.
   
   Testing this bug is also somewhat complicated, because `PathToDirectory` is a static helper inside an anonymous namespace rather than a public API. In practice, we cannot test it directly in the current test suite.
   
   More importantly, this bug is only exercised through `GraphInfo::Load(const std::string& path)`: `Load()` first reads the graph metadata file, and then calls `PathToDirectory(path)`. So to reproduce this bug in a test, we need to drive the full `GraphInfo::Load` code path rather than test the helper in isolation.
   https://github.com/apache/incubator-graphar/blob/d535a437667c6e2bf28f7f376930b13ed5aed63d/cpp/src/graphar/graph_info.cc#L1405-L1416
   
   I can think of two possible solutions:
   - Set up a mock S3 server so that we can exercise the full workflow and reliably trigger this bug.
   - Expose this function — for example, move it into a namespace like `graphar::details` / `graphar::internal`, and avoid installing that header into the user system.
   
   Actually, I am not familiar with this kind of problem.
   
   Any advice? CC @yangxk1 


-- 
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: commits-unsubscribe@graphar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@graphar.apache.org
For additional commands, e-mail: commits-help@graphar.apache.org