Conversation
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Joao Eduardo Luis <joao@clyso.com>
| """Valid PASETO envelope but non-JSON payload.""" | ||
|
|
||
| def test_non_json_payload_raises(self, mock_config: Config) -> None: | ||
| assert mock_config.server is not None |
There was a problem hiding this comment.
do we need to assert the mock_config.server? mock_config.server is always not None
There was a problem hiding this comment.
can't recall, but I'm pretty sure this was to make the linter happy.
| class TestCapsSerialization: | ||
| """Caps -> JSON -> back roundtrip preserves the value.""" | ||
|
|
||
| def test_auth_caps_roundtrip(self) -> None: | ||
| original = _AuthCapsModel( | ||
| caps=AuthorizationCaps.BUILDS_CREATE | AuthorizationCaps.BUILDS_LIST_OWN, | ||
| ) | ||
| json_str = original.model_dump_json() | ||
| restored = _AuthCapsModel.model_validate_json(json_str) | ||
| assert original.caps == restored.caps | ||
|
|
||
| def test_routes_caps_roundtrip(self) -> None: | ||
| original = _RoutesCapsModel( | ||
| caps=RoutesCaps.ROUTES_BUILDS_NEW | RoutesCaps.ROUTES_AUTH_LOGIN, | ||
| ) | ||
| json_str = original.model_dump_json() | ||
| restored = _RoutesCapsModel.model_validate_json(json_str) | ||
| assert original.caps == restored.caps | ||
|
|
||
| def test_auth_caps_all_roundtrip(self) -> None: | ||
| original = _AuthCapsModel.model_validate({"caps": ["all"]}) | ||
| json_str = original.model_dump_json() | ||
| restored = _AuthCapsModel.model_validate_json(json_str) | ||
| assert original.caps == restored.caps |
There was a problem hiding this comment.
Do we test a library functions? (model_dump_json and model_validate_json)
There was a problem hiding this comment.
these tests were auto-generated, but regardless this is a valid test hence why it was submitted. This is not validating model_dump_json nor model_validate_json, but the serialization and deserialization of all -- this is a meta flag that gets translated by _caps_from_str_lst() into the various AuthorizationCaps, which is itself an enum.
| @pytest.fixture | ||
| def default_perms() -> Permissions: | ||
| return permissions_from_yaml(_DEFAULT_YAML) |
There was a problem hiding this comment.
should this go into conftest.py, because it is a fixture?
There was a problem hiding this comment.
I'm not sure, not quite as proficient with pytest as I used to be. This seemed fine, given this was meant for permissions solely, not for all the tests?
| @pytest.fixture | ||
| def basic_perms(self) -> Permissions: | ||
| auth = Permissions(groups={}, rules=[]) | ||
| auth.groups["admin"] = AuthorizationGroup( | ||
| name="admin", | ||
| authorized_for=[ | ||
| ProjectAuthorizationEntry( | ||
| pattern=r".*", | ||
| caps=AuthorizationCaps.PROJECT_MANAGE | ||
| | AuthorizationCaps.PROJECT_LIST | ||
| | AuthorizationCaps.BUILDS_CREATE | ||
| | AuthorizationCaps.BUILDS_REVOKE_ANY | ||
| | AuthorizationCaps.BUILDS_LIST_ANY, | ||
| ), | ||
| RegistryAuthorizationEntry(pattern=r".*"), | ||
| RepositoryAuthorizationEntry(pattern=r".*"), | ||
| ], | ||
| ) | ||
| auth.groups["development"] = AuthorizationGroup( | ||
| name="development", | ||
| authorized_for=[ | ||
| ProjectAuthorizationEntry( | ||
| pattern=r"^dev/.*$", | ||
| caps=AuthorizationCaps.PROJECT_LIST | ||
| | AuthorizationCaps.BUILDS_CREATE | ||
| | AuthorizationCaps.BUILDS_LIST_OWN | ||
| | AuthorizationCaps.BUILDS_LIST_ANY | ||
| | AuthorizationCaps.BUILDS_REVOKE_OWN, | ||
| ), | ||
| RegistryAuthorizationEntry(pattern=r"^registry\.example\.com/dev/.*$"), | ||
| RepositoryAuthorizationEntry(pattern=r"https?://github\.com/clyso/.*"), | ||
| ], | ||
| ) | ||
| auth.groups["all"] = AuthorizationGroup( | ||
| name="all", | ||
| authorized_for=[ | ||
| ProjectAuthorizationEntry( | ||
| pattern=r".*", | ||
| caps=AuthorizationCaps.PROJECT_LIST | ||
| | AuthorizationCaps.BUILDS_LIST_OWN | ||
| | AuthorizationCaps.BUILDS_LIST_ANY, | ||
| ), | ||
| ], | ||
| ) | ||
| auth.rules.extend( | ||
| [ | ||
| UserAuthorizationRule(user_pattern=r"^.*@domain\.tld$", groups=["all"]), | ||
| UserAuthorizationRule( | ||
| user_pattern=r"^foo.bar@domain.tld$", | ||
| groups=["development"], | ||
| ), | ||
| UserAuthorizationRule( | ||
| user_pattern=r"^foo.baz@domain.tld$", | ||
| groups=["admin"], | ||
| ), | ||
| ] | ||
| ) | ||
| return auth | ||
|
|
There was a problem hiding this comment.
shouldn't this fixture be part of the conftest.py?
There was a problem hiding this comment.
but it's only used for this test? Again, not as proficient with pytest as I used to be, but I would assume conftest.py would only hold fixtures that are reused across the tests, instead of being the go-to place to dump even one-time-use fixtures.
Adds pytest support to the project, and introduces some tests for
cbsd/.This way we're moving away from kludge tests in modules'
__main__functions, and into something a lot more predictable and usable.This patch set was generated mostly by Claude Opus 4.6, with guidance by the submitter.
Signed-off-by: Joao Eduardo Luis <joao@clyso.com>