adutra commented on PR #3927: URL: https://github.com/apache/polaris/pull/3927#issuecomment-4016871790
> @adutra We used to try (imperfectly) to adhere to having a comprehensive covering of privileges always enumerated in those Authz tests before https://github.com/apache/polaris/pull/3824/changes#top but now I guess in the interest of reducing verbosity of the tests we no longer enumerate all the subsuming privileges so I it's less obvious to developers what should be required to be added to the tests. but this might be something to discuss more deeply on the mailing list - for something like Authz tests we probably _want_ to force ourselves to be explicit about testing which things succeed and which things fail whenever we create a new super-privilege. Auto-generating the "insufficient" sets is probably still a good thing because it could actually help reduce human error, _if_ we don't auto-generate subsuming-privilege "shouldPassWith" tests, since that way the tests can automatically identify all branches that need to be considered when defining the privilege. Yes, I added two "syntactic sugars": - All single-privilege negative tests are added automatically (by subtracting all the single-privilege positive tests) - All subsuming privilege positive tests are added automatically for each single-privilege positive test So in most cases, it's enough to explicitly declare the "least privilege" positive tests, and the test factory will derive the other tests from that. I think that generating negative tests automatically is a good thing and should not be removed. It actually helped me identify a few missing positive tests. Automatically generating the subsuming positive tests is imho also a good thing. Since the subsuming privileges are statically defined in `PolarisAuthorizerImpl`, I don't see much value in listing the subsuming test cases extensively for each test. But I'm open to revisiting this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
