mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-08 18:31:36 +01:00
Complete AL review knowledge readiness
Fill telemetry and Query coverage, strengthen thin review domains, correct audited content defects, and add deterministic cheap-model evaluation and reference-integrity safeguards. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
This commit is contained in:
parent
809af9708e
commit
e81632b4be
103 changed files with 2350 additions and 210 deletions
|
|
@ -38,14 +38,16 @@ Discard files that are not applicable. Retain conditionally applicable files (an
|
|||
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
|
||||
|
||||
- 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(...)]`, `[HandlerFunctions(...)]`, handler attributes, `asserterror`, `ExpectedError`, `ExpectedErrorCode`, fixture initialization, and test-library calls.
|
||||
- Tokens extracted from the diff that relate to testing (`Subtype = Test`, `TestIsolation`, `TransactionModel`, `AutoRollback`, `AutoCommit`, `Commit`, `asserterror`, `ExpectedError`, `ExpectedErrorCode`, `HandlerFunctions`, `ConfirmHandler`, `MessageHandler`, `StrMenuHandler`, `ModalPageHandler`, `Enqueue`, `Dequeue`, `AssertEmpty`, `Library Assert`, `LibraryVariableStorage`, `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, `Init`, `Insert`).
|
||||
- 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`, `Enqueue`, `Dequeue`, `AssertEmpty`, `Library Assert`, `LibraryVariableStorage`, `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, `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.
|
||||
|
||||
The following targeted checks cover every current `testing` 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 method in a `Subtype = Test` codeunit adds or changes `[TransactionModel(...)]`, exercises code that calls `Commit`, defaults broadly to `AutoCommit`, or uses `AutoCommit` (or exercises a path that calls `Commit`) without a `TestIsolation`-enabled runner — `transactionmodel-attribute-governs-test-transactions`. Do not worklist this article solely because an ordinary `AutoRollback` or read-only test has no `TestIsolation` runner.
|
||||
- A method in a `Subtype = Test` codeunit adds or changes `[TransactionModel(...)]`, exercises code that calls `Commit` under `AutoRollback`, defaults broadly to `AutoCommit`, or chooses `None` for a writing test — `transactionmodel-attribute-governs-test-transactions`.
|
||||
- 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`.
|
||||
- A test path raises UI, `[HandlerFunctions(...)]` does not exactly match the invoked handlers, a handler hardcodes replies instead of using enqueue/dequeue expectations, or `LibraryVariableStorage.Clear`/`AssertEmpty` is missing — `ui-handlers-in-tests`.
|
||||
|
|
@ -60,7 +62,7 @@ For each worklist entry, evaluate the diff against the file's `## Best Practice`
|
|||
|
||||
- When the diff contains a clear match for an Anti Pattern, emit a finding with severity `major` or `blocker`, a message summarizing the anti-pattern, `location` pointing to the offending line or range, and a `references` entry pointing to the knowledge file. Use `blocker` only when the test can pass while verifying the wrong behavior or can leave committed data that contaminates later tests; otherwise the ceiling is `major`.
|
||||
- When the diff contains code that contradicts a Best Practice without being a full anti-pattern, emit `minor` with the same reference shape.
|
||||
- When the skill cannot detect a violation but the file is clearly applicable to the change, emit `info` citing the file.
|
||||
- Applicability alone is not a finding. Emit `info` only for a concrete, non-actionable observation the article explicitly defines; otherwise emit nothing when no violation is present.
|
||||
|
||||
Set `confidence` to:
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue