chore: migrate to pyproject.toml, drop orphaned test.py#23
Merged
Conversation
Move static project metadata to pyproject.toml (PEP 621). setup.py is slimmed to just the C extension, whose net-snmp libraries are still discovered dynamically via net-snmp-config and can't be declared statically. Also remove netsnmp/tests/test.py: it is the legacy monolithic bindings test, not run by CI (the tests/system/ suite superseded it) and not matched by the unittest discover glob. The stale test_suite= reference in setup.py pointing at it is dropped along with the old dead distutils/ string imports and broken --basedir parsing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KG1c9XtKawkvig5PnZjC8
Markus Häll has contributed to the project (including the memory ownership fixes in netsnmp_walk) and is added alongside Christian Svensson in the project maintainers metadata. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KG1c9XtKawkvig5PnZjC8
Collaborator
|
I tested the PR with both Python 3.7 and Python 3.6 using the PEP 517 build path. Python 3.7 works correctly:
Python 3.6 fails while creating the isolated build environment. This is because The declared minimum version should therefore be changed from: requires-python = ">=3.6"to: requires-python = ">=3.7"I also found a less obvious test coverage regression in the removal of
The current system-test suite only covers SNMPv1 and SNMPv2c. Removing the legacy file therefore removes all existing SNMPv3 coverage. I suggest adding an SNMPv3 user to the test |
Addresses PR review feedback: - requires-python is raised from >=3.6 to >=3.7. The build-system pin of setuptools>=61 cannot be satisfied on 3.6, so the PEP 517 build failed while creating the isolated build env. - Removing the legacy netsnmp/tests/test.py dropped the only SNMPv3 coverage, as the system suite only exercised v1/v2c. Add an SNMPv3 authPriv (SHA/AES) user to the test snmpd config and a test_v3.py covering get, getnext, getbulk, set, walk and the session API. Unlike the legacy test, which only printed results, these assert. A negative case with a bad AuthPass proves the agent really enforces authPriv, so the positive tests cannot silently pass unauthenticated. Also ignore __pycache__/*.pyc, which were previously untracked-but-noisy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KG1c9XtKawkvig5PnZjC8
The wrong-password test passed authentication because net-snmp caches localized USM keys per (engineID, username) for the process lifetime. An earlier test had already authenticated as testuser with the correct passphrase, so the cached key was reused and the bad passphrase never reached the agent -- the test was asserting on the cache, not the agent. Use a dedicated 'baduser' that no passing test ever authenticates as, so its cache entry is only ever populated from the wrong passphrase. Also add an unknown-user case, which is rejected agent-side and so cannot be affected by client caching at all. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014KG1c9XtKawkvig5PnZjC8
snmp_duplicate_objid() allocates the securityAuthProto and
securityPrivProto OIDs, but they were never released. snmp_sess_open()
duplicates them into the session it returns, so the local copies are the
caller's to free -- they leaked on every SNMPv3 session creation
(1440 bytes over 18 blocks across the new v3 tests).
Caught by Valgrind once the new test_v3.py gave the v3 code path its
first coverage. Freeing at the 'end' label also covers the early goto
paths, where an unsupported privacy protocol returns after the
authentication protocol OID was already duplicated. session is {0}
initialised, so free() on the unset paths is a no-op.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014KG1c9XtKawkvig5PnZjC8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Modernizes packaging and removes dead test code.
pyproject.toml migration
pyproject.toml(PEP 621): name, version, description, readme, license, authors/maintainers, URLs, classifiers.setup.pyis slimmed to just thenetsnmp.client_intfC extension. Its net-snmp libraries are discovered dynamically vianet-snmp-config(including the Windowsshwrapper +ws2_32), which can't be expressed declaratively — this is the standard hybrid pyproject+setup.py pattern for C extensions.setup.py: the shadoweddistutils.coreimport, unusedstringimport, and the broken py2-era--basedirparsing.Remove
netsnmp/tests/test.pyIt's the legacy monolithic bindings test and is not executed by CI:
system-tests.ymlrunspython3 -m unittest discover -s netsnmp/tests/system -p 'test_*.py';test.pyis in the parent dir and doesn't matchtest_*.py.test_suite = "netsnmp.tests.test"in setup.py (fired only by the removed-in-modern-setuptoolssetup.py test), which is dropped here.The structured
netsnmp/tests/system/suite supersedes and expands its coverage, and is what actually runs (plus Valgrind, ASan/UBSan, and gcov coverage).Validation
Packaging is exercised by existing CI:
windows.yml(MSYS2 build) andsystem-tests.yml(build + full test suite under sanitizers) both build viasetup.py/backend on this branch.🤖 Generated with Claude Code