Unit tests for wifiap

[rmescandon]

• Added unit tests for wifiap operations and for basic http client ones also.
• wifi-ap command gets response or err and prints out the result. Printing was previously made in wifiap package, but now it returns values or error so that the package can be used in a command, in a test or in other situations requiring return values
• The http transport is mocked according to the test

[knitzsche]

Can you please add a README_TESTS doc that states how to run the tests?

[knitzsche]

should the defer() [1] come before the return (line 77) to ensure the response is closed even if there is an error? (probably before the if err != nill line)

https://golang.org/pkg/net/http/#Client.Do says: " If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close." [1] https://github.com/rmescandon/UCWifiConnect/blob/664740e808458cd513c2000643d69b6e7821838a/src/wifiap/restclient.go#L80

[knitzsche]

very small item: can you add the invalid url to the error string here: https://github.com/rmescandon/UCWifiConnect/blob/664740e808458cd513c2000643d69b6e7821838a/src/wifiap/restclient.go#L80

[knitzsche]

Tiny typo "Methog": https://github.com/rmescandon/UCWifiConnect/blob/664740e808458cd513c2000643d69b6e7821838a/src/wifiap/wifiap_test.go#L238

[knitzsche]

same typo in a few places "Methog"

[knitzsche]

just a thought: the cmdwifi-ap.go requires a passphrase of 13 chars (which is arbitrary I think the standard requires 8 chrs). But the backend SetPassphrase(...) does not. probably the validation should occur in SetPassphrase() only so it is enforced every time. And we could unit test on valid length.

[knitzsche]

I see you moved printMapSorted(). I wanted to use that from netman but did I not want to import wifi-ap pkg. I wonder if we should move it to a utils pkg (and Export it).

[knitzsche]

LGTM after these minor points are considered.

[knitzsche]

I realize I Approved, but I have one more possible issue: Do we need TWO N/newRestClient functions?

1) https://github.com/rmescandon/UCWifiConnect/blob/92f2888bc161c25f7ddc19b739301c25fcee3b8c/src/wifiap/wifiap.go#L33 2) https://github.com/rmescandon/UCWifiConnect/blob/92f2888bc161c25f7ddc19b739301c25fcee3b8c/src/wifiap/restclient.go#L52

That is, can you just export 2) and delete 1)?

same question for D/defaultRestclient: 1) https://github.com/rmescandon/UCWifiConnect/blob/92f2888bc161c25f7ddc19b739301c25fcee3b8c/src/wifiap/wifiap.go#L38 2) https://github.com/rmescandon/UCWifiConnect/blob/92f2888bc161c25f7ddc19b739301c25fcee3b8c/src/wifiap/restclient.go#L60

[rmescandon]

Not sure what you mean here. Client struct uses RestClient one. It is a composition. Client implements API methods and RestClient the transport operations. I don't see how to get rid of the builder method of Client to use the RestClient builder instead. What can be done is reducing both to only one struct (what is not a bad idea either, though it is a different design view). If you prefer that approach I would do it in another PR, as involves several changes