Skip to content

Add testing knowledge: UI handlers, table relations, asserterror, fixtures (P1+P2) - #62

Merged
Jesper Schulz-Wedde (JesperSchulz) merged 3 commits into
mainfrom
jesperschulz-testing-knowledge-p1-p2
Jul 9, 2026
Merged

Jesper Schulz-Wedde (JesperSchulz) merged 3 commits into
mainfrom
jesperschulz-testing-knowledge-p1-p2

Conversation

@JesperSchulz

@JesperSchulz Jesper Schulz-Wedde (JesperSchulz) commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds six BC-platform-specific testing-domain knowledge articles under microsoft/knowledge/testing/, each as a trio (<slug>.md + <slug>.good.al + <slug>.bad.al) — 18 files total. Every article clears the README admission test: a modern LLM reviewing or generating BC test code would get the mechanic wrong without it. None drift into generic test-writing advice (AAA, naming, one-assert-per-test).

Originally drafted in community/knowledge/testing/; relocated to the microsoft/ layer per maintainer request (pure git mv, no content changes).

The six articles & admission-test rationale

P1 trio

  • ui-calls-require-test-handlers — UI calls (Confirm, Message, Page.RunModal, StrMenu, Notification.Send, …) need a registered handler + [HandlerFunctions], or the test aborts with an unhandled-UI runtime error — not an assertion failure. LLMs routinely omit handlers.
  • tablerelation-requires-prerequisite-recordsValidate/Insert(true) enforces TableRelation; fixtures must be built parent-before-child or the test aborts on a relation error. LLMs assemble test data bottom-up.
  • handlers-enqueue-never-assert — handlers run in a separate context; an assertion inside a handler can be swallowed, so the test passes while broken. Capture via LibraryVariableStorage.Enqueue, assert in the body, finish with AssertEmpty. Non-obvious platform behavior.

P2 trio

  • handlerfunctions-attribute-must-match-ui-path — two-sided rule: a missing handler → unhandled UI; an unused listed handler → "handler function was not executed". Both are runtime failures. LLMs get the bidirectional contract wrong.
  • asserterror-needs-expectederror-and-code — bare asserterror passes on any error; pair it with Assert.ExpectedError + Assert.ExpectedErrorCode to verify the specific failure. LLMs treat asserterror as sufficient.
  • use-library-codeunits-for-test-fixturesLibrarySales/LibraryInventory/… create valid records (number series, mandatory fields, relations); hand-rolled Init/Insert skips required setup and rots as the schema evolves. LLMs invent Init/Insert fixtures.

Articles mirror the format/frontmatter of the existing microsoft/knowledge/testing/transactionmodel-attribute-governs-test-transactions article (6-field frontmatter, domain: testing, bc-version: [all], technologies: [al], countries: [w1], application-area: [all]; code in sibling .al files only). Each .md is 24–26 lines (well under the 100-line cap).

Validator outputs

$ python .github/scripts/validate_frontmatter.py --root .
Validator: 0 error(s), 0 warning(s)        (exit 0)

$ pwsh .github/scripts/Test-KnowledgeIndex.ps1 -Root .
BCQuality index: 198 article(s). ...
Knowledge-index check PASSED: 198 articles, deterministic, full coverage, selection inputs intact.   (exit 0)

$ pwsh ./tools/Build-KnowledgeIndex.ps1
BCQuality index: 198 article(s). Index: .../knowledge-index.json
198        (exit 0)

Article count invariant

Baseline on main: 192 articles → after adding 6: 198 articles. Delta = +6. (The community→microsoft relocation is a move, so it leaves the count unchanged at 198.)

CODEOWNERS note

These files now live under microsoft/knowledge/testing/, which CODEOWNERS gates with the testing domain experts (Nikola Kukrika (@nikolakukrika) ventselartur Bugsy (@pchriste-microsoft-com)). The PR therefore auto-requests them and is expected to sit BLOCKED on the 2-approval ruleset until they review. CODEOWNERS was not edited.


Do not merge — open for maintainer review.

Jesper Schulz-Wedde and others added 2 commits June 29, 2026 16:10
…tures (P1+P2)

Six BC-specific testing-domain knowledge articles in community/knowledge/testing/, each with .good.al/.bad.al samples:

- ui-calls-require-test-handlers
- tablerelation-requires-prerequisite-records
- handlers-enqueue-never-assert
- handlerfunctions-attribute-must-match-ui-path
- asserterror-needs-expectederror-and-code
- use-library-codeunits-for-test-fixtures

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Relocates the six P1+P2 testing-domain articles (18 files: .md + .good.al + .bad.al each) from community/knowledge/testing/ to microsoft/knowledge/testing/ per maintainer request. Pure git-mv rename; no content or frontmatter changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.good.al Outdated
Comment thread microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md Outdated
[ConfirmHandler]
procedure ConfirmHandlerYes(Question: Text; var Reply: Boolean)
begin
Reply := true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still a bad example. We should use the LibraryVariableStorage.Dequeue and assert if the question is as we expect it. Only part of the question should be checked, not entire text. Then we should dequeue the reply and provide it. This way we ensure that we get the right handler and that it is raised on the exact amount of times we expect it to be raised.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked into the merged ui-handlers-in-tests article: the test enqueues the expected question and reply, and ConfirmHandler dequeues, asserts via Assert.ExpectedConfirm (substring), then returns the enqueued reply. AssertEmpty() enforces the exact call count. (373ec31)

Comment thread microsoft/knowledge/testing/handlerfunctions-attribute-must-match-ui-path.good.al Outdated
Comment thread microsoft/knowledge/testing/handlerfunctions-attribute-must-match-ui-path.md Outdated
Comment thread microsoft/knowledge/testing/handlers-enqueue-never-assert.md Outdated
Comment thread microsoft/knowledge/testing/handlers-enqueue-never-assert.md Outdated
SalesLine.Validate("No.", 'GHOST');
SalesLine.Insert(true);

Assert.AreEqual('GHOST', SalesLine."No.", 'Unreachable: validation already failed.');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid hardcoding the Ghost value inline.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moot now — this article was dropped; the sample no longer exists. (373ec31)

Comment thread microsoft/knowledge/testing/tablerelation-requires-prerequisite-records.md Outdated
Comment thread microsoft/knowledge/testing/ui-calls-require-test-handlers.md Outdated
Respond to @nikolakukrika's review on #62:

- Merge ui-calls-require-test-handlers, handlerfunctions-attribute-must-match-ui-path
  and handlers-enqueue-never-assert into a single ui-handlers-in-tests article.
- Adopt the enqueue-from-test / dequeue-and-assert-in-handler pattern using
  Assert.ExpectedConfirm/ExpectedMessage (substring match), with Initialize()
  clearing LibraryVariableStorage and AssertEmpty() proving exact call counts.
- asserterror sample now uses Assert.ExpectedTestFieldError + FieldCaption instead
  of hardcoded message/code; article text points to the library helpers.
- Drop the tablerelation article and fold its test-relevant ordering point
  (relations checked on Validate/Insert(true); build parents first) into
  use-library-codeunits-for-test-fixtures.

Article count 198 -> 195.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@JesperSchulz

Copy link
Copy Markdown
Contributor Author

Nikola Kukrika (@nikolakukrika) thanks for the detailed review — pushed 373ec31 addressing all of it. Summary of the rework:

Consolidation. The three overlapping handler/UI articles (ui-calls-require-test-handlers, handlerfunctions-attribute-must-match-ui-path, handlers-enqueue-never-assert) are now a single ui-handlers-in-tests article, so the duplicated Confirm/Message bad samples are gone.

Handler pattern reversed to the library convention. The merged sample now has the test enqueue the expected text and reply, and the handlers dequeue + verify with Assert.ExpectedConfirm / Assert.ExpectedMessage (substring match, not the full caption) before returning the enqueued reply. Each test starts with an Initialize() that calls LibraryVariableStorage.Clear() and ends with AssertEmpty() to prove the exact interaction count. This intentionally retires the original "never assert in a handler" stance in favor of the library-driven pattern.

No hardcoded messages/codes. The asserterror sample now uses Assert.ExpectedTestFieldError(Customer.FieldCaption(Name), '') instead of a hardcoded message + 'TestField' code, and the article text points at the Library Assert helpers and recommends extending the library rather than hardcoding. (It assumes ExpectedTestFieldError lands in Library Assert — happy to align once you've moved it.)

Dropped the table-relation article. Agreed it wasn't test-specific enough; its one test-relevant point (a TableRelation field is checked on Validate/Insert(true), so build parents before children) is folded into use-library-codeunits-for-test-fixtures.

Counts. 198 → 195 articles. validate_frontmatter.py, Test-KnowledgeIndex.ps1, and Build-KnowledgeIndex.ps1 all pass.

I've replied inline on each thread. One open item for you: whether you're comfortable retiring the standalone "never assert in a handler" guidance now that it's expressed through the enqueue/dequeue pattern. Ready for another look when you are.

@JesperSchulz
Jesper Schulz-Wedde (JesperSchulz) merged commit 3aa3581 into main Jul 9, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants