diff --git a/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.bad.al b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.bad.al index 555506c..f068342 100644 --- a/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.bad.al +++ b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.bad.al @@ -7,6 +7,6 @@ codeunit 50130 "Sample Item Ledger Lookup" begin InvoiceNo := LibrarySales.PostSalesDocument(SalesHeader, true, true); ItemLedgerEntry.SetRange("Document No.", InvoiceNo); - ItemLedgerEntry.FindSet(); + if ItemLedgerEntry.FindSet() then; end; } diff --git a/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.good.al b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.good.al index 791b015..f396d9b 100644 --- a/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.good.al +++ b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.good.al @@ -8,6 +8,6 @@ codeunit 50130 "Sample Item Ledger Lookup" LibrarySales.PostSalesDocument(SalesHeader, true, true); ShippingNo := SalesHeader."Last Shipping No."; ItemLedgerEntry.SetRange("Document No.", ShippingNo); - ItemLedgerEntry.FindSet(); + if ItemLedgerEntry.FindSet() then; end; } diff --git a/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.md b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.md index d03ec12..3b5d939 100644 --- a/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.md +++ b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.md @@ -17,10 +17,10 @@ Posting a sales order with both Ship and Invoice in one call creates the Item Le After posting a sales order with Ship and Invoice together, read `SalesHeader."Last Shipping No."` (populated during the post) and filter Item Ledger Entry by that value, not by the invoice number the posting routine returns. -See sample: `item-ledger-entry-document-no-follows-last-shipping-no.good.al`. +See sample: [`item-ledger-entry-document-no-follows-last-shipping-no.good.al`](item-ledger-entry-document-no-follows-last-shipping-no.good.al). ## Anti Pattern Filtering Item Ledger Entry by the posted sales invoice number after a combined Ship-and-Invoice post. The filter compiles and runs without error but matches zero rows, because the entry belongs to the shipment leg of the posting, not the invoice leg. -See sample: `item-ledger-entry-document-no-follows-last-shipping-no.bad.al`. +See sample: [`item-ledger-entry-document-no-follows-last-shipping-no.bad.al`](item-ledger-entry-document-no-follows-last-shipping-no.bad.al). diff --git a/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.md b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.md index 5ff2992..e06f738 100644 --- a/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.md +++ b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.md @@ -11,23 +11,23 @@ application-area: [all] ## Description -A fixture helper that assigns a hardcoded literal to a primary-key or descriptive field collides the moment two tests, or two runs of the same test, create that fixture without cleanup, and a literal longer than the field allows raises a truncation or insert error. `LibraryUtility.GenerateGUID()` is not a real GUID — it is a `Code[10]` number-series value (`GU00000000`–`GU99999999`) — and it returns the full 10 characters unshortened. Truncating it yourself with `CopyStr(..., 1, MaxStrLen(ShorterField))` for a field under 10 characters is unsafe: the changing digits sit at the right end and are exactly what gets cut off, so consecutive calls into a short field can produce the same truncated value. `GenerateGUID()` is only safe as-is for a field that holds the full 10 characters. +A fixture helper that assigns a hardcoded literal to a primary-key field, or to any field the test relies on as a unique lookup identifier, collides the moment two tests, or two runs of the same test, create that fixture without cleanup — and a literal longer than the field allows raises a truncation or insert error. An ordinary descriptive field carries no such constraint: two rows with the same description do not collide on insert, and a deterministic descriptive value is often exactly what an exact-match assertion needs, so none of this applies to it. `LibraryUtility.GenerateGUID()` is not a real GUID — it is a `Code[10]` number-series value (`GU00000000`–`GU99999999`) — and it returns the full 10 characters unshortened. Truncating it yourself with `CopyStr(..., 1, MaxStrLen(ShorterField))` for a field under 10 characters is unsafe: the changing digits sit at the right end and are exactly what gets cut off, so consecutive calls into a short field can produce the same truncated value. `GenerateGUID()` is only safe as-is for a field that holds the full 10 characters. ## Best Practice For a field that holds the full 10 characters, assign `LibraryUtility.GenerateGUID()` directly. For a shorter field, do not truncate a GUID yourself — but also do not assume every `LibraryUtility` helper verifies uniqueness against the real table, because they don't all behave the same way: -- `GenerateRandomCode(FieldNo, TableNo)` opens the target table as a **temporary** `RecordRef`, so its own emptiness check never inspects real rows — despite taking `TableNo`, it does not verify against the actual table. It's safe to use for its non-colliding-*within-a-single-test-run* value (derived from `GenerateGUID()`'s own number series), not for a guarantee against pre-existing or leftover data. +- `GenerateRandomCode(FieldNo, TableNo)` opens the target table as a **temporary** `RecordRef`: the buffer starts and stays empty, so its `repeat...until RecRef.IsEmpty()` loop always exits after one iteration — despite taking `TableNo`, it never checks the real table, and it never retries even within its own call. Its value is the rightmost `FieldRef.Length` characters of `GenerateGUID()`'s sequential `GU00000000`–`GU99999999` series, so for a short field that window of digits cycles: a 1-character field repeats every 10 calls, a 2-character field every 100, and so on. It is a finite short-field namespace with a low collision *chance* within one test run — not a guarantee at any scope, unlike the table-checking helpers below. - `GenerateRandomCodeWithLength(FieldNo, TableNo, CodeLength)` opens the real (non-temporary) table and loops until the generated value doesn't collide — a genuine verified-unique guarantee — but it returns `Code[10]` regardless of the requested `CodeLength`, so it's only useful for a field of 10 characters or fewer. - `GenerateRandomCode20(FieldNo, TableNo)` is the same real, verified-against-the-table pattern as `GenerateRandomCodeWithLength`, sized for a `Code[20]` field. - `GenerateRandomXMLText(Length)` performs no table lookup at all — it's a plain random-text generator, appropriate for a descriptive/incidental field where uniqueness doesn't matter, not for a value that needs to be collision-checked. Pick `GenerateRandomCodeWithLength`/`GenerateRandomCode20` when the test genuinely needs a code verified unique against the table; use `GenerateRandomCode`/`GenerateGUID`/`GenerateRandomXMLText` for incidental values where a low collision *chance* is enough. -See sample: `use-generateguid-for-unique-test-fixture-values.good.al`. +See sample: [`use-generateguid-for-unique-test-fixture-values.good.al`](use-generateguid-for-unique-test-fixture-values.good.al). ## Anti Pattern -Hardcoding a fixture value such as `'TEST001'` or a short descriptive literal, which collides across parallel or repeated test runs. Equally an anti-pattern: truncating `GenerateGUID()`'s result with `CopyStr(..., 1, MaxStrLen(Field))` for a field shorter than 10 characters — the truncation removes the part of the value that actually varies. +Hardcoding a primary-key or unique-lookup fixture value such as `'TEST001'`, which collides across parallel or repeated test runs — a fixed descriptive value is not this anti-pattern, since the field carries no uniqueness constraint. Equally an anti-pattern: truncating `GenerateGUID()`'s result with `CopyStr(..., 1, MaxStrLen(Field))` for a field shorter than 10 characters — the truncation removes the part of the value that actually varies. -See sample: `use-generateguid-for-unique-test-fixture-values.bad.al`. +See sample: [`use-generateguid-for-unique-test-fixture-values.bad.al`](use-generateguid-for-unique-test-fixture-values.bad.al). diff --git a/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.md b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.md index a1f95d3..f94c792 100644 --- a/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.md +++ b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.md @@ -17,10 +17,10 @@ Whether a field can actually be changed is a distinct state from whether it is s Open the `TestPage` with `OpenEdit()`, navigate to the relevant record, then assert against `TestPageField.Editable()` to verify whether the field can be changed under the given precondition. -See sample: `use-testpage-editable-to-verify-field-editability.good.al`. +See sample: [`use-testpage-editable-to-verify-field-editability.good.al`](use-testpage-editable-to-verify-field-editability.good.al). ## Anti Pattern Asserting `Enabled()` (or checking nothing at all) when the actual claim is about editability, or opening the page with `OpenView()` when the field's editability depends on business logic that only applies in edit mode. -See sample: `use-testpage-editable-to-verify-field-editability.bad.al`. +See sample: [`use-testpage-editable-to-verify-field-editability.bad.al`](use-testpage-editable-to-verify-field-editability.bad.al). diff --git a/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.md b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.md index d40979f..ff6bd86 100644 --- a/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.md +++ b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.md @@ -17,10 +17,10 @@ A UI test codeunit does not need to inspect table or page properties indirectly Open the `TestPage`, navigate to the relevant record if needed, then assert against `TestPageField.Visible()` and `TestPageField.Enabled()` to verify the field's shown/enabled state, rather than checking an unrelated table/page property or skipping the check. -See sample: `use-testpage-visible-enabled-to-verify-field-ui-state.good.al`. +See sample: [`use-testpage-visible-enabled-to-verify-field-ui-state.good.al`](use-testpage-visible-enabled-to-verify-field-ui-state.good.al). ## Anti Pattern A test that opens the `TestPage` but never asserts against `Visible()`/`Enabled()` on the field in question — confirming only that the page opens, not that the field behaves as expected. -See sample: `use-testpage-visible-enabled-to-verify-field-ui-state.bad.al`. +See sample: [`use-testpage-visible-enabled-to-verify-field-ui-state.bad.al`](use-testpage-visible-enabled-to-verify-field-ui-state.bad.al). diff --git a/microsoft/skills/review/al-data-modeling-review.md b/microsoft/skills/review/al-data-modeling-review.md index ec3d7fb..ca0682f 100644 --- a/microsoft/skills/review/al-data-modeling-review.md +++ b/microsoft/skills/review/al-data-modeling-review.md @@ -16,7 +16,7 @@ application-area: [all] Reviews AL source changes against the `data-modeling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`. -An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, or audit fields. The skill returns `not-applicable` when none of those apply. +An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, dimension wiring, journal-based posting-routine structure, or Item Ledger Entry document-number lookups after a combined sales post. The skill returns `not-applicable` when none of those apply. ## Source @@ -84,7 +84,7 @@ Outcome selection: - `completed` — the skill evaluated every worklist item. - `no-knowledge` — no applicable data-modeling knowledge survived filtering. -- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface. +- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, audit-field, dimension-wiring, posting-routine-structure, or Item-Ledger-Entry-document-number surface. - `partial` — a budget was hit before the worklist was exhausted. - `failed` — an unrecoverable error occurred. diff --git a/microsoft/skills/review/al-testing-review.md b/microsoft/skills/review/al-testing-review.md index 6399062..884408e 100644 --- a/microsoft/skills/review/al-testing-review.md +++ b/microsoft/skills/review/al-testing-review.md @@ -50,7 +50,7 @@ The following targeted checks cover every current `testing` article. Treat each - 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`. -- Test fixture code assigns a hardcoded literal to a primary-key or descriptive field, hand-builds a "unique" value (string concatenation, a counter, `Format(CurrentDateTime)`), or truncates `LibraryUtility.GenerateGUID()`'s result with `CopyStr` for a field shorter than 10 characters — `use-generateguid-for-unique-test-fixture-values`. Calling `GenerateGUID()` untruncated into a full-length field, or `GenerateRandomCodeWithLength`/`GenerateRandomCode20` for a shorter field needing real verified uniqueness, is the compliant shape, not the signal to flag. Do not claim `GenerateRandomCode` (without `WithLength`/`20`) or `GenerateRandomXMLText` verify uniqueness against the real table — they don't. +- Test fixture code assigns a hardcoded literal to a primary-key field or a field the test relies on as a unique lookup identifier, hand-builds a "unique" value for such a field (string concatenation, a counter, `Format(CurrentDateTime)`), or truncates `LibraryUtility.GenerateGUID()`'s result with `CopyStr` for such a field shorter than 10 characters — `use-generateguid-for-unique-test-fixture-values`. Calling `GenerateGUID()` untruncated into a full-length field, or `GenerateRandomCodeWithLength`/`GenerateRandomCode20` for a shorter field needing real verified uniqueness, is the compliant shape, not the signal to flag. A hardcoded or deterministic value in an ordinary descriptive field is not this anti-pattern — that field carries no uniqueness constraint. Do not claim `GenerateRandomCode` (without `WithLength`/`20`) or `GenerateRandomXMLText` verify uniqueness against the real table, or that `GenerateRandomCode` is collision-free even within one test run for a short field — none of that is true. - A test asserts against a `TestPage` field's `.Visible()` or `.Enabled()` — `use-testpage-visible-enabled-to-verify-field-ui-state`. When the assertion is against `.Editable()`, or the page is opened with `OpenEdit()` specifically to check editability — `use-testpage-editable-to-verify-field-editability`. - A test path raises UI and `[HandlerFunctions(...)]` does not match the invoked handlers, or the test has no meaningful evidence of the UI result (for example, it treats a Boolean set before the action as proof of success) — `ui-handlers-in-tests`. A capture/reset/assert-after-`RunModal` pattern is valid. Enqueue/dequeue and `AssertEmpty` are required only when order, count, text, replies, or a scripted sequence is part of the contract. Only nonoptional handlers have to execute: a listed handler declared `[SendNotificationHandler(true)]` or `[RecallNotificationHandler(true)]` is optional by design, so do not treat it as unmatched when the run never raises the notification.