Created attachment 247436 [details] trivial update of Makefile and distinfo
bumping this
anyone? Thank you
Created attachment 248427 [details] Add tests - Add pytest tests OK 847 passed, 204 skipped in 3.49s Do you mind take a look? I my opinion is always good to have a working unittest for debug proposes and it doesn't affect port since we just use TEST_DEPENDS.
Hi Nuno, thanks a lot for the feedback, I am happy to add support for the test suite for this, but as you can see here, running make tests includes quite some dependencies, including ldap-tests module, that is not in the ports tree [1] [1] https://github.com/hvac/hvac/blob/main/pyproject.toml#L61 Have you got anything against porting it? I can work on it if you are ok Thank you
(In reply to Alessandro Sagratini from comment #4) TEST_ARGS= --ignore=tests/integration_tests/api/auth_methods/test_ldap.py is included to fix ldap dep.
(In reply to Nuno Teixeira from comment #5) (...) I've got inspired by Arch linux pkg that for it seems don't have py-ldap ported too but have tests on check target. I will commit your patch and leave this PR open until you decide to add tests to port. Cheers
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=c4533a8ac7659afc6ceba7319bdf614406c4b996 commit c4533a8ac7659afc6ceba7319bdf614406c4b996 Author: Alessandro Sagratini <ale_sagra@hotmail.com> AuthorDate: 2024-02-13 13:29:40 +0000 Commit: Nuno Teixeira <eduardo@FreeBSD.org> CommitDate: 2024-02-13 13:34:45 +0000 devel/py-hvac: Update to 2.1.0 ChangeLog: https://github.com/hvac/hvac/releases/tag/v2.1.0 PR: 276103 devel/py-hvac/Makefile | 2 +- devel/py-hvac/distinfo | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
fair, if I add these lines to the Makefile TEST_DEPENDS= ${PYTHON_PKGNAMEPREFIX}parameterized>0:devel/py-parameterized@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}requests-mock>=0:www/py-requests-mock@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}pytest-mock>0:devel/py-pytest-mock@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}werkzeug>=0:www/py-werkzeug@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}jwcrypto>=0:security/py-jwcrypto@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}authlib>=0:security/py-authlib@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}flask-sqlalchemy>=0:databases/py-flask-sqlalchemy@${PY_FLAVOR} \ vault>=1.14.0:security/vault TEST_ARGS= --ignore=tests/integration_tests/api/auth_methods/test_ldap.py USES= python USE_PYTHON= autoplist pep517 pytest and then run make test, I get: 1444 passed, 52 skipped, 21 warnings in 51.87s skipped tests are mostly Vault-related ones, like "SKIPPED [1] tests/integration_tests/api/secrets_engines/test_transform.py:282: Transform secrets engine only supported with Enterprise Vault" Is this more reasonable? Thank you
I am happy to create a new patch to add tests :)
Comment on attachment 248427 [details] Add tests I would just add vault>=1.14.0:security/vault line to TEST_DEPENDS, other than that, the patch looks fine and I am happy to include it
(In reply to Alessandro Sagratini from comment #10) Nice. I missed some deps from Arch pkg: depends=(python python-requests) makedepends=(python-build python-installer python-poetry-core vault python-pyhcl) checkdepends=(python-pytest python-authlib python-flask python-flask-sqlalchemy python-parameterized python-requests-mock python-werkzeug python-jwcrypto python-pytest-mock python-pytest-xdist consul) optdepends=( 'python-pyhcl: for HCL parser' ) You already added vault and it improved results. They have "consul" as dep too. In our ports, sysutils/consul (Service discovery and configuration made easy) and some python related. Do you mind test to see if it influences?
(In reply to Nuno Teixeira from comment #11) (...) + consul>0:sysutils/consul
if I also add consul to TEST_DEPENDS, the outcome is: 1 failed, 1447 passed, 48 skipped, 21 warnings in 51.95 the error is: E AssertionError: 'test/' unexpectedly found in {'cubbyhole/': {'accessor': 'cubbyhole_b5d98348', 'config': {'default_lease_ttl': 0, 'force_no_cache': False, 'max_lease_ttl': 0}, 'description': 'per-token private secret storage', 'external_entropy_access': False, 'local': True, 'options': None, 'plugin_version': '', 'running_plugin_version': 'v1.14.1+builtin.vault', 'running_sha256': '', 'seal_wrap': False, 'type': 'cubbyhole', 'uuid': '27c041da-f74b-70d1-0a86-a7e296778b92'}, 'identity/': {'accessor': 'identity_32b95fd2', 'config': {'default_lease_ttl': 0, 'force_no_cache': False, 'max_lease_ttl': 0, 'passthrough_request_headers': ['Authorization']}, 'description': 'identity store', 'external_entropy_access': False, 'local': False, 'options': None, 'plugin_version': '', 'running_plugin_version': 'v1.14.1+builtin.vault', 'running_sha256': '', 'seal_wrap': False, 'type': 'identity', 'uuid': '590dd5db-f067-2720-d686-ccfa8d285e3d'}, 'kvv1_mount/': {'accessor': 'kv_0c53b402', 'config': {'default_lease_ttl': 0, 'force_no_cache': False, 'max_lease_ttl': 0}, 'deprecation_status': 'supported', 'description': '', 'external_entropy_access': False, 'local': False, 'options': {'version': '1'}, 'plugin_version': '', 'running_plugin_version': 'v0.15.0+builtin', 'running_sha256': '', 'seal_wrap': False, 'type': 'kv', 'uuid': 'b9e1b315-a010-1fa7-3d49-a9d547b80923'}, 'sys/': {'accessor': 'system_e0180af3', 'config': {'default_lease_ttl': 0, 'force_no_cache': False, 'max_lease_ttl': 0, 'passthrough_request_headers': ['Accept']}, 'description': 'system endpoints used for control, policy and debugging', 'external_entropy_access': False, 'local': False, 'options': None, 'plugin_version': '', 'running_plugin_version': 'v1.14.1+builtin.vault', 'running_sha256': '', 'seal_wrap': True, 'type': 'system', 'uuid': '6a8af665-2a0a-64b8-b764-f359afca6c4b'}, 'test/': {'accessor': 'kv_4f33aea8', 'config': {'default_lease_ttl': 3600, 'force_no_cache': False, 'max_lease_ttl': 8600}, 'deprecation_status': 'supported', 'description': '', 'external_entropy_access': False, 'local': False, 'options': None, 'plugin_version': '', 'running_plugin_version': 'v0.15.0+builtin', 'running_sha256': '', 'seal_wrap': False, 'type': 'kv', 'uuid': '00ab2ee5-3d76-628b-cd93-ff5eb0b6ece7'}} tests/integration_tests/v1/test_system_backend.py:115: AssertionError
(In reply to Alessandro Sagratini from comment #13) Ok, I'm building all test deps as pkgs so I can get faster tests under poudriere jail. Tomorrow morning I will get into this PR again and see what we have to add tests to port. Cheers
Created attachment 248436 [details] Add RUN_TESTS I am adding a new patch for tests, feel free to test it and modify accordingly when you have time
(In reply to Alessandro Sagratini from comment #15) Got nice results with same deps as your latest patch: 1448 passed, 48 skipped, 21 warnings Summary of skipped tests: .. Legacy MFA support dropped in Vault <1.11.0 .. Vault 1.13.0 dropped this feature. .. Transform secrets engine only supported with Enterprise Vault .. Namespaces only supported with Enterprise Vault .. EGP policies only supported with Enterprise Vault .. Older versions of Vault return different JSON structure I think we good on quality tests. Notes: - No errors - Adding devel/py-pyhcl, didn't change results. - Tested on 15-CURRENT poudriere jail
(In reply to Nuno Teixeira from comment #16) (...) My tests deps differs from yours: ${PYTHON_PKGNAMEPREFIX}authlib>0:security/py-authlib@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}flask-sqlalchemy>0:databases/py-flask-sqlalchemy@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}flask>0:www/py-flask@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}jwcrypto>0:security/py-jwcrypto@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}parameterized>0:devel/py-parameterized@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}pytest-mock>0:devel/py-pytest-mock@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}pytest-xdist>0:devel/py-pytest-xdist@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}requests-mock>0:www/py-requests-mock@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}werkzeug>0:www/py-werkzeug@${PY_FLAVOR} \ vault>=1.14.0:security/vault \ consul>0:sysutils/consul These are the same as Arch pkg and are less than your patch lists.
Created attachment 248457 [details] Add tests - shorter test depends list IMO we should move on with this shorter list since adding more deps to it doesn't bring better results as all tests run OK.
(In reply to Nuno Teixeira from comment #18) I agree, if adding a dependency doesn't improve results, we should not put it there, unless absolutely required :)
(In reply to Alessandro Sagratini from comment #19) Ok, let's commit it. In future hvac versions, if you encounter tests failures, just add a comment after TEST_DEPENDS: TEST_DEPENDS= .. # Tests failing, url to upstream PR/issue (if available) These way users can consult it. I will keep this PR open for a week just in case we need some changes to it. Cheers
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=7d0368bea459f6ea8cc21f5417e565e688ebec1b commit 7d0368bea459f6ea8cc21f5417e565e688ebec1b Author: Nuno Teixeira <eduardo@FreeBSD.org> AuthorDate: 2024-02-14 11:56:33 +0000 Commit: Nuno Teixeira <eduardo@FreeBSD.org> CommitDate: 2024-02-14 11:58:36 +0000 devel/py-hvac: Add tests PR: 276103 devel/py-hvac/Makefile | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
Committed, thanks!
all changes including tests were submitted, closing