Skip to content

Conversation

@doxxx
Copy link

@doxxx doxxx commented Oct 31, 2025

I'm opening this PR to initiate discussion around solutions for #1691. The solution implemented here solves #1691 because it is relatively easy to mock a method that returns Stream. However, this solution prevents access to the Timeout property of SftpFileStream. It is also not binary-backwards-compatible but somewhat source-backwards-compatible.

@Rob-Hague
Copy link
Collaborator

Thanks for the PR. My thoughts are basically the same as in #1508 - that changing popular public API for the sake of mocking is just not worth it at this point.

The impact of this one could potentially be lessened by changing only the interface and explicitly implementing the methods on the concrete client, but I'd still not really be convinced

@doxxx
Copy link
Author

doxxx commented Nov 5, 2025

Do you have any suggestions for how to solve the mocking issue? Right now the only viable solution is #1691 (comment), which is not terrible but still a little sad that it's necessary.

@Rob-Hague
Copy link
Collaborator

IMO the wrapper is totally valid since it provides the most control and flexibility. I think it would be the same approach as that in which one wanted to mock a component that called methods on System.IO.File, or System.Net.Http.HttpClient etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants