knowledge: improve review precision from BCApps PR 10010 feedback

This commit is contained in:
github-actions[bot] 2026-09-09 12:35:07 +00:00
parent 8584217c75
commit e0805a9875

View file

@ -19,10 +19,12 @@ What matters is the effective permission context at the moment the protected ope
Use `TestPermissions::Restrictive` for a permission-sensitive test and lower the current test user with the test framework's `"Permissions Mock"` or `"Library - Lower Permissions"` before invoking the protected operation. Assign a permission context that actually contains the rights the scenario tests — either the permission set itself or a role that includes it — and restore or stop the mock afterward. Use `Disabled` only for suites that do not assert permission behavior, or where the test lowers the context explicitly through the test libraries instead of relying on the runner. Do not require a test to apply the permission set under test directly when it reaches the same rights through a composed role and then asserts the boundary. Use `TestPermissions::Restrictive` for a permission-sensitive test and lower the current test user with the test framework's `"Permissions Mock"` or `"Library - Lower Permissions"` before invoking the protected operation. Assign a permission context that actually contains the rights the scenario tests — either the permission set itself or a role that includes it — and restore or stop the mock afterward. Use `Disabled` only for suites that do not assert permission behavior, or where the test lowers the context explicitly through the test libraries instead of relying on the runner. Do not require a test to apply the permission set under test directly when it reaches the same rights through a composed role and then asserts the boundary.
For web-service/API E2E suites driven through `Library - Graph Mgt` (or an equivalent client-request wrapper), the protected page or trigger executes on the separate web-service session identity, not on the test codeunit's own session. `Permissions Mock` and `Library - Lower Permissions` only lower the test session and therefore cannot reach the `ReadPermission`/`WritePermission` gates evaluated on that other session — applying them would not exercise anything real. `TestPermissions = Disabled` with no permission lowering is correct for this pattern; do not flag it as a missing-lowered-context gap. Genuinely exercising those gates would require a restricted user authenticating on the web-service session, which is a different (and out of scope) test setup.
See sample: `permission-tests-must-lower-the-execution-context.good.al`. See sample: `permission-tests-must-lower-the-execution-context.good.al`.
## Anti Pattern ## Anti Pattern
Setting `TestPermissions = Disabled` or leaving the effective D365 Full Access context in place while asserting that a limited user is denied, or adding a `[TestPermissions(...)]` attribute without any runner/test-library code that applies the intended permission set. Do not report the mirror image: a test that lowers the context through a role including the permission set under test, and then asserts the boundary, has exercised that permission set and is not a coverage gap. Setting `TestPermissions = Disabled` or leaving the effective D365 Full Access context in place while asserting that a limited user is denied, or adding a `[TestPermissions(...)]` attribute without any runner/test-library code that applies the intended permission set. Do not report the mirror image: a test that lowers the context through a role including the permission set under test, and then asserts the boundary, has exercised that permission set and is not a coverage gap. Also do not report a `Library - Graph Mgt` (or equivalent web-service client) E2E suite that runs `TestPermissions = Disabled` with no permission lowering — the protected operation runs on the web-service session, which the test session's mock cannot reach, so lowering the test session would be a no-op.
See sample: `permission-tests-must-lower-the-execution-context.bad.al`. See sample: `permission-tests-must-lower-the-execution-context.bad.al`.