search

vsecades / AbuseIpDb

AbuseIpDB - Wrapper around the AbuseIPDb service API
MIT License
15 stars 4 forks source link

Add basic tests for regression #7

Closed Eorhim closed 4 years ago

Eorhim commented 4 years ago

I should have added them right at the start. Unfortunately I introduced an API change with my refactoring. Still I think, it is much cleaner than the print statements.

These tests will be the safety net for all future changes. They uncover that check_cidr and report_ip can create malformed URL. As Abuse IP DB set the sunset date for APIv1 to 2020-02-01, it doesn't make sense to fix that.

vsecades commented 4 years ago

@Eorhim thanks for the contributions. I got a response from AbuseIPDB that they want to publish this alongside their service, so would be good to get this wrapped up in the next week or so, if you have the time.

I'll take a look at the changes for V2 to see how to move forward.

Eorhim commented 4 years ago

Hello Valentin, is there still something missing from my side? Did I overlook some requested change? My fork is now in a state, where it fits my needs. I'm also satisfied with the test coverage and structure. Feel free to pull the changes. If you do so, either merge develop directly or merge the branches develop-add-tests, develop-add-new-class-interface and develop-add-apiv2 in that order. They're based on each other. I'm happy to hear suggestions for improvement. If I don't hear from you, I'll move on to the next part in my project. As I'm offline for the next few days, I wish you a habby new year! Eorhim

vsecades commented 4 years ago

Hi Sebastian,

If you have the chance to talk, that would be great. Reach out to me via Skype at vsecades, or we can find some other means of communication.

If you can make a pull request with the latest changes, that would be great as well.

Thanks again for your help.

Cheers, Valentin

On Fri, Dec 27, 2019, 11:54 AM Sebastian Bonhag notifications@github.com wrote:

Hello Valentin, is there still something missing from my side? Did I overlook some requested change? My fork is now in a state, where it fits my needs. I'm also satisfied with the test coverage and structure. Feel free to pull the changes. If you do so, either merge develop directly or merge the branches develop-add-tests, develop-add-new-class-interface and develop-add-apiv2 in that order. They're based on each other. I'm happy to hear suggestions for improvement. If I don't hear from you, I'll move on to the next part in my project. As I'm offline for the next few days, I wish you a habby new year! Eorhim

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/vsecades/AbuseIpDb/pull/7?email_source=notifications&email_token=AAGTDCBWOR2TTBHUT5G2Y53Q2Y6L7A5CNFSM4J7JO4V2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHXRG2Y#issuecomment-569316203, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGTDCHPQVSBNWRNTRYEXVDQ2Y6L7ANCNFSM4J7JO4VQ .

Eorhim commented 4 years ago

@vsecades As I wrote in the README.md, only the functionality for bulk_upload is missing. And I forgot one thing: decoding the JSON and the tests for that. I'm unsure, if I should leave it at that or create a specific response class ...

vsecades commented 4 years ago

Please leave as it was. This was a boilerplate item that we can use at a later date.

Hi @Eorhim this is one of the pending items :-)