Test compatibility with macOS

charlestolley / python-snmp

A user-friendly SNMP library

MIT License

15 stars 3 forks source link

Test compatibility with macOS #9

Closed denisbondar closed 6 months ago

denisbondar commented 6 months ago

Hi, @charlestolley !

I think I've found a library that at first glance fulfills all my wishes and requirements. And I am very grateful to you!

I'd like to contribute to it in a small way, but before I start, I have to ask if there is some preferred way to propose changes via PR?

Maybe we should first discuss the changes being made before actually making them (for this, it's probably better to activate discussions in the repository).

In any case, some code changes need to be made before I can proceed:

the test method uses os.getrandom, which is only available in Linux. Since this is just a test, maybe it would be better to use something more universal?
another of the tests was a failure because it uses the snmp.openssl module, which is actually missing (there is only a stub file).

Thanks!

charlestolley commented 6 months ago

Hi, thanks for the feedback, I'm glad you like what you are seeing. I didn't realize os.getrandom() was only available in Linux, I've changed that test to use random.randint() instead. As for the test that uses snmp.openssl, the test file is written in such a way that the test should not run if there is no security implementation defined. I don't think I have documented this, but the "official" way to run the tests (i.e. the way I do it and can confirm that it works) is to run python -m test from the top-level directory of the repository. Can you tell me how you are running the tests? I'd be curious to know why your method doesn't work.

denisbondar commented 6 months ago

Hi! That's how it is! You made me question the validity of my launch command :)

The thing is that I haven't used the unittest module for a long time and all my tests in all my projects have been rewritten to pytest for a long time. However, from old memory, when I still had everything on unittest, I always ran the unuttest module like this: python -m unittest, but now I remembered that I added the discover option to make the unittest module find all my files. Anyway, if you just run python -m unittest, the openssl test fails, but the others all succeed.

The method you suggested: python -m test works fine!

Thank you!

Can you tell me how you accept PRs? Do you have to create an issue for discussion beforehand?

charlestolley commented 6 months ago

I don't have a formal process for accepting PRs, because so far I haven't had anyone seriously interested in contributing. I think it would be a good idea to open an issue to discuss anything you want to change. I've rejected PRs in the past because the requestor did not really understand what they were changing.

denisbondar commented 6 months ago

Then it's probably best to just write a separate issue for each question. Although, I only have one issue so far. I will open an issue for it.