mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-06 07:06:54 +01:00
Address second round of Jesper Schulz-Wedde's review on PR #158
- use-generateguid-for-unique-test-fixture-values.md/.good.al: documented each LibraryUtility helper's actual behavior, verified against LibraryUtility.Codeunit.al. GenerateRandomCode opens the target table as a temporary RecordRef, so despite taking TableNo it never checks real data. GenerateRandomXMLText performs no table lookup at all. Only GenerateRandomCodeWithLength/GenerateRandomCode20 (capped at Code[10]/ Code[20]) genuinely verify against the real table. Fixture switched to GenerateRandomCodeWithLength where the comment claims verified uniqueness. - al-testing-review.md: rewired the cue to catch the actual anti-pattern (hardcoded literals, hand-built uniqueness, short-field GUID truncation) instead of only matching the compliant GenerateGUID()+CopyStr shape; broadened tokens to include TestPage, Library - Utility, and .Visible()/.Enabled()/.Editable(). - al-data-modeling-review.md: restricted the Item Ledger Entry Last-Shipping-No. cue to sales combined posting; purchase combined posting is Receive+Invoice and uses different fields entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
e538664a50
commit
26871856ee
4 changed files with 19 additions and 10 deletions
|
|
@ -6,13 +6,15 @@ codeunit 50132 "Sample Customer Type Library"
|
|||
procedure CreateCustomerType(var CustomerType: Record "Customer Type")
|
||||
begin
|
||||
CustomerType.Init();
|
||||
// Code is shorter than GenerateGUID()'s 10 characters, so use
|
||||
// GenerateRandomCode instead of truncating a GUID ourselves — it
|
||||
// verifies uniqueness against the table rather than just returning
|
||||
// a truncated slice of the number series.
|
||||
CustomerType.Code := LibraryUtility.GenerateRandomCode(CustomerType.FieldNo(Code), Database::"Customer Type");
|
||||
// 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, so no uniqueness verification is needed here.
|
||||
// untruncated, and only needs to be incidental, not verified-unique.
|
||||
CustomerType.Description := CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(CustomerType.Description));
|
||||
CustomerType.Insert(true);
|
||||
end;
|
||||
|
|
|
|||
|
|
@ -15,7 +15,14 @@ A fixture helper that assigns a hardcoded literal to a primary-key or descriptiv
|
|||
|
||||
## Best Practice
|
||||
|
||||
For a field that holds the full 10 characters, assign `LibraryUtility.GenerateGUID()` directly. For a shorter or arbitrary-length field, use `LibraryUtility.GenerateRandomCode(FieldNo, TableNo)` (or `GenerateRandomCodeWithLength`/`GenerateRandomXMLText(Length)` for a specific length) instead of truncating a GUID yourself — these generate the value and verify it is actually unique against the target table, rather than relying on the number series alone.
|
||||
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`.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue