honor httpAgent/httpsAgent when setting ntlm flag (take 2)

[smokhov]

• Closes #1209

This PR supersedes PR #1209 (by @unilynx) by adding tests for 'ntlm' to be together with httpAgent and httpsAgent. I used two different approaches of creating the tests agents inspired from other tests where they were used and by making sure ntlm was set to true. I've also sync'd it with master.

[smokhov]

@w666 -- so I've done some testing here. TL;DR, the proposes change overall makes sense. To test it i had to dig a bit. Now the question to you would be how would make this available from scope of tests. Once solved, then can fix the tests to query for the agents.

I've pushed the instrumentation commit a54e32d. Most of some of it to be reverted once solution is found. I've doubled the number of request-response tests with and without change based on the options, and they all pass, but if I console-log the http(s)Agent at the right spot, they are

Summary:

• https.ts instantiates either AxiosInstance or NtlmClient stored in req depending on the options ntlm or not. https://github.com/vpulim/node-soap/blob/79a03631c6a38fb7665dfd2281f3a5f0fbd82d58/src/http.ts#L182
• the OG proposed change, { httpAgent: exoptions.httpAgent, httpsAgent: exoptions.httpsAgent } passes this as a config parameter to NtlmClient at the constructor (axios-ntlm config). I actually tested and think a better way to pass all of the options, (I kept them commented out):

      const ntlmReq = NtlmClient(
        {
          username: exoptions.username,
          password: exoptions.password,
          workstation: exoptions.workstation || '',
          domain: exoptions.domain || '',
        },
        // Change wanted in the OG PR:
        //{ httpAgent: exoptions.httpAgent, httpsAgent: exoptions.httpsAgent },
        // A better change per axios-ntlm API?
        //options,
      );

• then actually the next line has req = ntlmReq(options); so supposedly the agents would have been passed, but this call appears to be a Promise call and the agents information seems not to be passed further properly. Doing it explicitly and eagerly at the constructor appears to change the agent properly from my logs
• to test for this properly we need to examine the resulting req object from the request() call https://github.com/vpulim/node-soap/blob/79a03631c6a38fb7665dfd2281f3a5f0fbd82d58/src/http.ts#L179
• currently the return value from that call is inhibited through HttpClient._invoke()in _defineMethod(): https://github.com/vpulim/node-soap/blob/79a03631c6a38fb7665dfd2281f3a5f0fbd82d58/src/client.ts#L263 https://github.com/vpulim/node-soap/blob/79a03631c6a38fb7665dfd2281f3a5f0fbd82d58/src/client.ts#L576

If we can get access to that return value of reqat the scope of the tests (I could not find an easy way to do so), then we can compare the http(s)Agent values from the actual to the requested and fail if they don't match by commenting out the change. I am currently ouf of energy to dig deeper, so if you have insights and suggestions, please feel free. Otherwise, the change makes sense as I said.

[w666]

I do remember about this one, just did not have time for it yet.

[w666]

Okay, what I think is we can ignore integration tests for this and just run some manual testing (I think @smokhov already did it). Unfortunately, same cases are hard to test, especially with this particular code without refactoring it.

I will do a quick manual check too and I think I will just merge that other PR with the actual code changes.