-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes #17
base: master
Are you sure you want to change the base?
Changes #17
Conversation
Version 1.1.8.0
Use different unifisites on one connection simultaneously.
Hi @Master-Silver, thanks for sharing your work! If I understand it correctly, it's adding a fakes framework to allow unit testing without having a UniFi controller present to test against, correct? While I certainly do see how this could be useful as the current UnitTests would be changing device settings and changing them back again on a live UniFi controller, it doesn't seem to be complete and adds some level of complexity into the mix. I'm not sure if that's worth it for such a simple project to be honest. I'll have a look at the 401s if I can find a decent way to deal with that. Thanks for sharing again and please correct me if I have misunderstand your contribution. |
Just added code that should deal with the 401 scenario you described. It's in the latest release. |
Hi @KoenZomers, yes the idea was to have unit tests that not need a real controller to work. I use your API in my project and the other components in my project have all unit tests (282 together at the moment). At some point of time it could be possible that I return to your API and write unit tests for the parts I use. I just wanted to know if you are interested in it. But either way I would definitely advise to get the Interface for the API, I needed it to write a fake API for my other components unit tests. Have a nice day |
Thanks for elaborating @Master-Silver. It makes sense what you're saying. I fixed the automated build for this project in GitHub and then also came to realize that the unit tests in the build could never work without having something like fakes. If time permits you, could you rebase this branch on the current master and implement it a bit further so I can see and learn from you how you would implement this? |
@KoenZomers, sure I really like to do that. At the moment I have to get the project I am working on to a stable release state, but after that there is a good change I have time to working on your API again. |
@Master-Silver Curious to hear if you have found a chance to continue with your proposal as discussed here. |
@KoenZomers sadly not, but I still intend to do that. Maybe there will soon be time for it, but at the moment I'm still busy with other parts of the project which I mentioned in my previous posts. |
Hi @KoenZomers, I found some time and added your changes on the master to my fork. I did make a new branch for the unit tests with fake classes. It was cleaner than to rebase on the old branch. Take a look https://github.com/Master-Silver/UniFiApi/tree/unit-tests-with-fakes |
Awesome! I'll have a look at it. Appreciate taking the time to create it. |
@KoenZomers had you any time to take a look? |
Not yet unfortunately, it's on my todo list. |
Hi,
just take a look on these changes. I use your API for a guest WLAN portal, but for my needs I had to do some adjustments on it. Maybe some of it will interest you. I'm still not finished with my work but one thing I noticed was, if the connection to the network controller is not used for some time, the controller will reject new requests with a 401 and there is no way of knowing before that happens, that you have to authenticate again.