Refactor logging to allow a loggerfactory per session

[desdesdes]

Refactor logging to allow a loggerfactory per session specified in the ConnectionInfo.

This commit introduces an ILoggerFactory to various classes, replacing the static logger factory with an instance-based approach for more flexible and session-specific logging. These changes improve the logging framework's flexibility and maintainability and allow unit testing of logging.

[Rob-Hague]

ah and unfortunately a lot of the mocking tests are "strict" in that they expect every interface call to be declared. I think you can do whatever is easiest to get them working i.e. remove the strictness if it helps

[desdesdes]

ah and unfortunately a lot of the mocking tests are "strict" in that they expect every interface call to be declared. I think you can do whatever is easiest to get them working i.e. remove the strictness if it helps

Removing the strictness and working around the null return would require a lot of ?? code in the library. So for now i chose the setup the logger everywhere. This create a lot of additions to the unit test, but I think this is the cleanest solution for now.

[Rob-Hague]

LGTM otherwise, although I have not tried it out to see if it addresses #1669 (I trust you have done that)

@mus65 any other comments?

[mus65]

LGTM otherwise, although I have not tried it out to see if it addresses #1669 (I trust you have done that)

@mus65 any other comments?

I will take a closer look on the weekend.

[mus65]

LGTM. I wrote a test case for this in a separate branch, but I'm not sure it's worth adding since it requires yet another dependency (test-only though). I would leave this up to @Rob-Hague .

This imho makes SshNetLoggingConfiguration kind of obselete, but marking it obsolete after just introducing it doesn't seem sensible. 😄

[Rob-Hague]

LGTM. I wrote a test case for this in a separate branch, but I'm not sure it's worth adding since it requires yet another dependency (test-only though). I would leave this up to @Rob-Hague .

That's a nice test to validate this change. I imagine it is less useful going forward (although perhaps moreso than some other tests...)