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 new file mode 100644 index 00000000..555506ca --- /dev/null +++ b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.bad.al @@ -0,0 +1,12 @@ +codeunit 50130 "Sample Item Ledger Lookup" +{ + procedure GetPostedItemLedgerEntries(var SalesHeader: Record "Sales Header"; var ItemLedgerEntry: Record "Item Ledger Entry") + var + LibrarySales: Codeunit "Library - Sales"; + InvoiceNo: Code[20]; + begin + InvoiceNo := LibrarySales.PostSalesDocument(SalesHeader, true, true); + ItemLedgerEntry.SetRange("Document No.", InvoiceNo); + ItemLedgerEntry.FindSet(); + 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 new file mode 100644 index 00000000..791b0152 --- /dev/null +++ b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.good.al @@ -0,0 +1,13 @@ +codeunit 50130 "Sample Item Ledger Lookup" +{ + procedure GetPostedItemLedgerEntries(var SalesHeader: Record "Sales Header"; var ItemLedgerEntry: Record "Item Ledger Entry") + var + LibrarySales: Codeunit "Library - Sales"; + ShippingNo: Code[20]; + begin + LibrarySales.PostSalesDocument(SalesHeader, true, true); + ShippingNo := SalesHeader."Last Shipping No."; + ItemLedgerEntry.SetRange("Document No.", ShippingNo); + ItemLedgerEntry.FindSet(); + 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 new file mode 100644 index 00000000..d03ec129 --- /dev/null +++ b/microsoft/knowledge/data-modeling/item-ledger-entry-document-no-follows-last-shipping-no.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [item-ledger-entry, document-no, last-shipping-no, ship-and-invoice, posting, sales-order] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# After Ship-and-Invoice posting, Item Ledger Entry carries the shipment document number + +## Description + +Posting a sales order with both Ship and Invoice in one call creates the Item Ledger Entry during the shipment leg of that combined post, so the entry's `Document No.` is stamped with the value assigned to the shipment — `Sales Header."Last Shipping No."` — not the posted sales invoice number the posting call returns. Code that filters Item Ledger Entry by the invoice number instead finds nothing: `SetRange`/`FindSet` simply return zero rows, with no error to signal the mistake. + +## Best Practice + +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`. + +## 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`. diff --git a/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.bad.al b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.bad.al new file mode 100644 index 00000000..c718bc7f --- /dev/null +++ b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.bad.al @@ -0,0 +1,10 @@ +codeunit 50132 "Sample Customer Type Library" +{ + procedure CreateCustomerType(var CustomerType: Record "Customer Type") + begin + CustomerType.Init(); + CustomerType.Code := 'TEST001'; + CustomerType.Description := 'Test Customer Type'; + CustomerType.Insert(true); + end; +} diff --git a/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.good.al b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.good.al new file mode 100644 index 00000000..25a194e2 --- /dev/null +++ b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.good.al @@ -0,0 +1,21 @@ +codeunit 50132 "Sample Customer Type Library" +{ + var + LibraryUtility: Codeunit "Library - Utility"; + + procedure CreateCustomerType(var CustomerType: Record "Customer Type") + begin + CustomerType.Init(); + // Code is shorter than GenerateGUID()'s 10 characters, and this field's + // uniqueness matters, so use GenerateRandomCodeWithLength: it opens the + // real (non-temporary) table and loops until the value doesn't collide. + // GenerateRandomCode would not do this — it opens the table as temporary, + // so its own emptiness check never inspects real rows. + CustomerType.Code := + LibraryUtility.GenerateRandomCodeWithLength(CustomerType.FieldNo(Code), Database::"Customer Type", MaxStrLen(CustomerType.Code)); + // Description is long enough to hold the full GenerateGUID() value + // untruncated, and only needs to be incidental, not verified-unique. + CustomerType.Description := CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(CustomerType.Description)); + CustomerType.Insert(true); + end; +} 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 new file mode 100644 index 00000000..5ff29928 --- /dev/null +++ b/microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.md @@ -0,0 +1,33 @@ +--- +bc-version: [all] +domain: testing +keywords: [generateguid, library-utility, test-fixtures, uniqueness, generaterandomcode, maxstrlen] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Generate unique test fixture values with LibraryUtility helpers, not hardcoded literals + +## 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. + +## 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. +- `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`. + +## 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. + +See sample: `use-generateguid-for-unique-test-fixture-values.bad.al`. diff --git a/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.bad.al b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.bad.al new file mode 100644 index 00000000..ddaf0ff0 --- /dev/null +++ b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.bad.al @@ -0,0 +1,27 @@ +codeunit 50134 "Sample Customer Type Edit Test" +{ + Subtype = Test; + + [Test] + procedure CustomerTypeFieldNotEditable_WhenLocked() + var + Assert: Codeunit Assert; + CustomerType: Record "Customer Type"; + CustomerTypeCard: TestPage "Customer Type Card"; + begin + // [GIVEN] a customer type record whose Locked flag is set + CustomerType.Init(); + CustomerType.Locked := true; + CustomerType.Insert(true); + + // [WHEN] the page is opened in VIEW mode — editability logic that only + // applies in edit mode is not exercised the same way + CustomerTypeCard.OpenView(); + CustomerTypeCard.GoToRecord(CustomerType); + + // [THEN] wrong function: Enabled() does not verify editability + Assert.IsFalse(CustomerTypeCard.Description.Enabled(), 'Description should not be editable while Locked is set.'); + + CustomerTypeCard.Close(); + end; +} diff --git a/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.good.al b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.good.al new file mode 100644 index 00000000..d683f0b5 --- /dev/null +++ b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.good.al @@ -0,0 +1,26 @@ +codeunit 50133 "Sample Customer Type Edit Test" +{ + Subtype = Test; + + [Test] + procedure CustomerTypeFieldNotEditable_WhenLocked() + var + Assert: Codeunit Assert; + CustomerType: Record "Customer Type"; + CustomerTypeCard: TestPage "Customer Type Card"; + begin + // [GIVEN] a customer type record whose Locked flag is set + CustomerType.Init(); + CustomerType.Locked := true; + CustomerType.Insert(true); + + // [WHEN] the page is opened in edit mode on that record + CustomerTypeCard.OpenEdit(); + CustomerTypeCard.GoToRecord(CustomerType); + + // [THEN] the field's actual editable state reflects the lock + Assert.IsFalse(CustomerTypeCard.Description.Editable(), 'Description should not be editable while Locked is set.'); + + CustomerTypeCard.Close(); + end; +} 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 new file mode 100644 index 00000000..a1f95d31 --- /dev/null +++ b/microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: testing +keywords: [testpage, editable, openedit, ui-state, field-verification] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Verify field editability with TestPage.Editable(), opened in edit mode + +## Description + +Whether a field can actually be changed is a distinct state from whether it is shown or enabled — `Editable()` and `Enabled()` are separate `TestField` functions. Verifying editability also requires opening the `TestPage` with `OpenEdit()`, not `OpenView()`: `OpenView()` opens the page in view mode, so it does not exercise the field's own conditional editability logic the way an actual edit-mode session does. + +## Best Practice + +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`. + +## 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`. diff --git a/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.bad.al b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.bad.al new file mode 100644 index 00000000..410af398 --- /dev/null +++ b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.bad.al @@ -0,0 +1,14 @@ +codeunit 50131 "Sample Customer Type UI Test" +{ + Subtype = Test; + + [Test] + procedure CustomerTypeFieldIsEnabledOnCustomerCard() + var + CustomerCard: TestPage "Customer Card"; + begin + // Confirms only that the page opens - never checks the field's actual UI state + CustomerCard.OpenView(); + CustomerCard.Close(); + end; +} diff --git a/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.good.al b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.good.al new file mode 100644 index 00000000..6e03c06e --- /dev/null +++ b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.good.al @@ -0,0 +1,15 @@ +codeunit 50131 "Sample Customer Type UI Test" +{ + Subtype = Test; + + [Test] + procedure CustomerTypeFieldIsEnabledOnCustomerCard() + var + Assert: Codeunit Assert; + CustomerCard: TestPage "Customer Card"; + begin + CustomerCard.OpenView(); + Assert.IsTrue(CustomerCard."Customer Type".Enabled(), 'Customer Type should be enabled on the Customer Card.'); + Assert.IsTrue(CustomerCard."Customer Type".Visible(), 'Customer Type should be visible on the Customer Card.'); + end; +} 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 new file mode 100644 index 00000000..d40979f2 --- /dev/null +++ b/microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: testing +keywords: [testpage, visible, enabled, ui-state, headless-test, field-verification] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Verify field visibility and enabled state with TestPage.Visible()/.Enabled() + +## Description + +A UI test codeunit does not need to inspect table or page properties indirectly to confirm a field is shown or enabled under given conditions. The `TestPage` object exposes a `Visible()` and an `Enabled()` function on each field, reflecting the page's actual rendered state, callable directly from a `[Test]` procedure. `Enabled()` and `Editable()` are distinct states — this article covers visibility/enabled state specifically; see `use-testpage-editable-to-verify-field-editability.md` for verifying whether a field can actually be changed. + +## Best Practice + +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`. + +## 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`. diff --git a/microsoft/skills/review/al-data-modeling-review.md b/microsoft/skills/review/al-data-modeling-review.md index 2386613a..0d6d01ec 100644 --- a/microsoft/skills/review/al-data-modeling-review.md +++ b/microsoft/skills/review/al-data-modeling-review.md @@ -46,6 +46,7 @@ A file enters the candidate worklist when its `keywords` intersect the extracted The following targeted checks cover every current `data-modeling` article. Treat each as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action. - A `* Setup` table or its page changes singleton structure, uses a nonblank or generated key, permits insert/delete, uses a List page, or does not ensure the blank-keyed row exists — `setup-table-is-a-singleton`. +- Code reads `Item Ledger Entry."Document No."` (or `"Last Shipping No."`/`"Last Posting No."`) after a combined Ship+Invoice **sales** post — `item-ledger-entry-document-no-follows-last-shipping-no`. This is a sales-specific rule: purchase combined posting is Receive+Invoice and uses receiving fields such as `"Last Receiving No."`, not the shipment/document-number behavior this article describes. Do not worklist it from purchase posting code. - A custom master table changes its primary key, `No.`/`No. Series` fields, or `OnInsert` without assigning a blank `No.` from setup through a number series — `master-table-no-from-number-series-in-oninsert`. - BC v22 or later code introduces or retains `NoSeriesManagement`, `InitSeries`, `SelectSeries`, or `SetSeries`, or number assignment/manual-entry checks do not use codeunit `"No. Series"` methods such as `GetNextNo`, `IsManual`, or `TestManual` — `use-no-series-codeunit-not-noseriesmanagement`. - A master gains or changes `Blocked`, or a document line, journal line, reference-field `OnValidate`, or posting routine uses that master without `TestField(Blocked, false)` at the point of use; also cue when the check is placed only in the master's own triggers — `check-blocked-in-referencing-code-not-in-master`. diff --git a/microsoft/skills/review/al-testing-review.md b/microsoft/skills/review/al-testing-review.md index 8bad7345..ca7b0130 100644 --- a/microsoft/skills/review/al-testing-review.md +++ b/microsoft/skills/review/al-testing-review.md @@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially codeunits with `Subtype = Test`, test runner codeunits with `TestIsolation`, test libraries, and codeunits that define UI handlers. - The changed methods and attributes, weighted toward `[Test]`, `[TransactionModel(...)]`, `[TestPermissions(...)]`, `[HandlerFunctions(...)]`, handler attributes, `asserterror`, `ExpectedError`, `ExpectedErrorCode`, fixture initialization, and test-library calls. -- Tokens extracted from the diff that relate to testing (`Subtype = Test`, `Subtype = TestRunner`, `TestIsolation`, `TestPermissions`, `Restrictive`, `NonRestrictive`, `Disabled`, `Permissions Mock`, `Library - Lower Permissions`, `TransactionModel`, `AutoRollback`, `AutoCommit`, `Commit`, `asserterror`, `ExpectedError`, `ExpectedErrorCode`, `HandlerFunctions`, `ConfirmHandler`, `MessageHandler`, `StrMenuHandler`, `ModalPageHandler`, `SendNotificationHandler`, `RecallNotificationHandler`, `Enqueue`, `Dequeue`, `AssertEmpty`, `Library Assert`, `LibraryVariableStorage`, `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, `Init`, `Insert`). +- Tokens extracted from the diff that relate to testing (`Subtype = Test`, `Subtype = TestRunner`, `TestIsolation`, `TestPermissions`, `Restrictive`, `NonRestrictive`, `Disabled`, `Permissions Mock`, `Library - Lower Permissions`, `TransactionModel`, `AutoRollback`, `AutoCommit`, `Commit`, `asserterror`, `ExpectedError`, `ExpectedErrorCode`, `HandlerFunctions`, `ConfirmHandler`, `MessageHandler`, `StrMenuHandler`, `ModalPageHandler`, `SendNotificationHandler`, `RecallNotificationHandler`, `Enqueue`, `Dequeue`, `AssertEmpty`, `Library Assert`, `LibraryVariableStorage`, `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, `Library - Utility`, `LibraryUtility`, `GenerateGUID`, `GenerateRandomCode`, `TestPage`, `.Visible(`, `.Enabled(`, `.Editable(`, `OpenNew`, `OpenView`, `OpenEdit`, `Init`, `Insert`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. When the diff contains no testing-related changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files. @@ -50,6 +50,8 @@ 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. +- 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. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.