diff --git a/microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md b/microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md index 4560a18..9ebf3f8 100644 --- a/microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md +++ b/microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md @@ -23,4 +23,6 @@ See sample: [`asserterror-needs-expectederror-and-code.good.al`](asserterror-nee `asserterror DoInvalid();` with nothing after it. The test asserts only that the call failed somehow; swap the validation for a different bug and the test still passes, certifying a guard that may no longer fire. A negative test that cannot tell one error from another verifies almost nothing. +Not an instance of this anti-pattern: a trailing `asserterror Error(SomeLabel)` used purely as an end-of-test rollback sentinel to undo a lazily-initialized shared fixture's scratch changes (see `commit-shared-test-fixture-inside-lazy-initialize.md`). That `Error` call exists to force a rollback, not to verify that a specific failure occurred — the sentinel's own text is not meant to be asserted against, and adding an `ExpectedError` there would just duplicate the label without checking anything the test doesn't already control. + See sample: [`asserterror-needs-expectederror-and-code.bad.al`](asserterror-needs-expectederror-and-code.bad.al). diff --git a/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.bad.al b/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.bad.al index 2c13967..f2af637 100644 --- a/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.bad.al +++ b/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.bad.al @@ -3,6 +3,7 @@ codeunit 50142 "Sample Test Library" Subtype = Test; var + LibraryInventory: Codeunit "Library - Inventory"; Initialized: Boolean; SharedItemNo: Code[20]; RollBackMsg: Label 'Revert back the tables to their original state.'; @@ -22,9 +23,7 @@ codeunit 50142 "Sample Test Library" var Item: Record Item; begin - Item.Init(); - Item."No." := 'SAMPLE-SHARED'; - Item.Insert(true); + LibraryInventory.CreateItem(Item); SharedItemNo := Item."No."; end; diff --git a/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.good.al b/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.good.al index 0879e6a..c550697 100644 --- a/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.good.al +++ b/microsoft/knowledge/testing/commit-shared-test-fixture-inside-lazy-initialize.good.al @@ -3,6 +3,7 @@ codeunit 50142 "Sample Test Library" Subtype = Test; var + LibraryInventory: Codeunit "Library - Inventory"; Initialized: Boolean; SharedItemNo: Code[20]; RollBackMsg: Label 'Revert back the tables to their original state.'; @@ -21,9 +22,7 @@ codeunit 50142 "Sample Test Library" var Item: Record Item; begin - Item.Init(); - Item."No." := 'SAMPLE-SHARED'; - Item.Insert(true); + LibraryInventory.CreateItem(Item); SharedItemNo := Item."No."; end; diff --git a/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.bad.al b/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.bad.al index 22fe8ed..1550350 100644 --- a/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.bad.al +++ b/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.bad.al @@ -1,3 +1,29 @@ +table 50144 "Sample Setup" +{ + fields + { + field(1; "Primary Key"; Code[10]) { } + field(2; "Default Category Code"; Code[20]) { } + } + keys + { + key(PK; "Primary Key") { Clustered = true; } + } +} + +table 50145 "Sample Header" +{ + fields + { + field(1; "No."; Code[20]) { } + field(10; "Category Code"; Code[20]) { } + } + keys + { + key(PK; "No.") { Clustered = true; } + } +} + codeunit 50141 "Sample Table Relation Test Ext" { [EventSubscriber(ObjectType::Codeunit, Codeunit::"Table Relation Test", 'OnAfterRemoveTableRelation', '', false, false)] diff --git a/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.good.al b/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.good.al index 8aa3ab8..0a6d480 100644 --- a/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.good.al +++ b/microsoft/knowledge/testing/table-relation-test-exclude-known-invalid-relations-via-event.good.al @@ -1,3 +1,32 @@ +table 50144 "Sample Setup" +{ + fields + { + field(1; "Primary Key"; Code[10]) { } + field(2; "Default Category Code"; Code[20]) { } + } + keys + { + key(PK; "Primary Key") { Clustered = true; } + } +} + +table 50145 "Sample Header" +{ + fields + { + field(1; "No."; Code[20]) { } + // A known exception: this field is allowed to reference "Sample + // Setup" loosely (no TableRelation enforced here on purpose), so + // the standard Table Relation Test would otherwise reject it. + field(10; "Category Code"; Code[20]) { } + } + keys + { + key(PK; "No.") { Clustered = true; } + } +} + codeunit 50141 "Sample Table Relation Test Ext" { [EventSubscriber(ObjectType::Codeunit, Codeunit::"Table Relation Test", 'OnAfterRemoveTableRelation', '', false, false)] diff --git a/microsoft/skills/review/al-testing-review.md b/microsoft/skills/review/al-testing-review.md index eaf73af..fdf5be0 100644 --- a/microsoft/skills/review/al-testing-review.md +++ b/microsoft/skills/review/al-testing-review.md @@ -49,7 +49,7 @@ The following targeted checks cover every current `testing` article. Treat each - An `AutoCommit` test runs under a `Subtype = TestRunner` codeunit that omits `TestIsolation` or sets it to `Disabled`, leaving committed data between tests — `testisolation-belongs-on-the-test-runner`. Require runner/repository context; a standalone test file cannot prove which runner executes it. - A permission-sensitive test uses `TestPermissions = Disabled`, claims to test a restricted user without `"Permissions Mock"`/`"Library - Lower Permissions"`, or declares `[TestPermissions(...)]` without applying that context — `permission-tests-must-lower-the-execution-context`. - Test fixture code manually calls `Init`/`Insert`, invents keys or prerequisite records, or bypasses available `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, or equivalent library codeunits — `use-library-codeunits-for-test-fixtures`. -- `asserterror` is added or changed without a following `Assert.ExpectedError`, `Assert.ExpectedErrorCode`, or a purpose-built assertion such as `ExpectedTestFieldError` — `asserterror-needs-expectederror-and-code`. Exclude `asserterror Assert.IsTrue(...)` / `asserterror Assert.IsFalse(...)` guarding a `Boolean`-returning call — that shape belongs to `use-assert-isfalse-not-asserterror-for-boolean-checks` instead, which wins for it. Also exclude a trailing `asserterror Error(...)` used purely as an end-of-test rollback sentinel after a lazy `Initialize()` fixture already committed — that shape belongs to `commit-shared-test-fixture-inside-lazy-initialize`, which wins for it; the sentinel's own error text is not meant to be asserted against. +- `asserterror` is added or changed without a following `Assert.ExpectedError`, `Assert.ExpectedErrorCode`, or a purpose-built assertion such as `ExpectedTestFieldError` — `asserterror-needs-expectederror-and-code`. Exclude `asserterror Assert.IsTrue(...)` / `asserterror Assert.IsFalse(...)` only when it is used solely to invert the guarded call's Boolean result (the same condition `use-assert-isfalse-not-asserterror-for-boolean-checks` cues on below, which wins for that shape) — not when the test expects the guarded Boolean-returning call itself to raise an error, which this rule still owns even though it happens to wrap an `Assert.IsTrue`/`IsFalse` call. Also exclude a trailing `asserterror Error(...)` used purely as an end-of-test rollback sentinel after a lazy `Initialize()` fixture already committed — that shape belongs to `commit-shared-test-fixture-inside-lazy-initialize`, which wins for it; the sentinel's own error text is not meant to be asserted against. - `asserterror` wraps `Assert.IsTrue(BooleanExpression, ...)` (or the `IsFalse` mirror) solely to invert the boolean result of the guarded call, rather than to assert that call itself raises an error — `use-assert-isfalse-not-asserterror-for-boolean-checks`. - A shared/lazy `Initialize()`-style fixture helper creates fixture data without a following `Commit()`, in a test method whose body later forces its own rollback (for example `asserterror Error(...)` used for end-of-test cleanup) — `commit-shared-test-fixture-inside-lazy-initialize`. The presence of `Commit()` after the fixture is the compliant shape, not the signal to look for; the missing-`Commit()` shape combined with a later deliberate rollback is the anti-pattern. Require runner/repository context for the `TestIsolation` value: a standalone test file cannot prove which runner executes it, and under `Function`-level isolation this whole pattern is moot regardless of `Commit()` — do not raise the finding when the executing runner's `TestIsolation` is known to be `Function`. - Changed code subscribes to `OnAfterRemoveTableRelation`, calls `RemoveTableRelation`, or references `Codeunit "Table Relation Test"`/134926 — `table-relation-test-exclude-known-invalid-relations-via-event`.