Commit graph

4 commits

Author SHA1 Message Date
Michael Dieringer
69b09db3a6 Fix remaining correctness issues from Jesper's 2026-09-15 re-review
- commit-shared-test-fixture-inside-lazy-initialize: three sub-issues.
  Recommended TestIsolation = Codeunit instead of listing Disabled as an
  equal option - Disabled never rolls back at all ("tests are not
  isolated from each other" per the property's own docs), so a fixture
  this pattern commits under Disabled is permanent database
  contamination unless something else tears it down; Disabled is now
  only mentioned alongside that explicit teardown requirement. Added
  precedence in al-testing-review.md so the deliberate end-of-test
  asserterror Error(...) rollback sentinel isn't also flagged by the
  generic asserterror-needs-expectederror-and-code rule. Rewrote both
  fixtures to actually demonstrate the pattern: persisted fixture data
  (an Item record) instead of an empty comment, a second [Test] method
  that depends on the fixture surviving into it, and an explicit
  Subtype = TestRunner / TestIsolation = Codeunit runner codeunit.

- table-relation-test-exclude-known-invalid-relations-via-event:
  the length/type rule was stated as one global requirement. Verified
  ValidateFieldRelation in codeunit 134926 directly (BCApps reference
  clone) and split it into the two branches the source actually has:
  a field with any unconditional relation needs exact length and exact
  resolved type; a field whose relations are all conditional only fails
  on being shorter (longer is fine) than the largest related field, and
  when the required type is specifically Code, a Text source passes too
  - a tolerance that does not apply on the unconditional side and does
  not extend to a required Text.

Rebased onto upstream/main (one conflict in
transactionmodel-attribute-governs-test-transactions.md - upstream had
already linked its sample references via the READ convention, ours
added a Source section; merged both). Also converted the 3 remaining
plain-backtick sample references in this PR to the READ-convention
markdown-link form, same fix as #156/#157/#158.
2026-09-21 22:49:39 +02:00
Michael Dieringer
1392521a8e Address second round of Jesper Schulz-Wedde's review on PR #159
- commit-shared-test-fixture-inside-lazy-initialize.md: fundamentally
  rewritten. AutoCommit is the documented default TransactionModel, not
  AutoRollback. Explains the real mechanism (Commit() protects a fixture
  from the test method's own later deliberate rollback, per Codeunit.Run/
  TransactionModel-property semantics) and the TestIsolation dependency
  (Disabled/Codeunit survive across methods, Function does not). Fixtures
  rewritten to demonstrate the actual failure/success shape.
- transactionmodel-attribute-governs-test-transactions.md: now states the
  AutoCommit default explicitly and agrees with the article above, closing
  the contradiction Jesper flagged between the two testing articles.
- Deleted confirm-needs-strsubstno-before-confirmhandler-sees-substituted-text
  (.md/.good.al/.bad.al): the underlying platform bug (microsoft/
  ALAppExtensions#23935) was closed as completed in Feb 2024; cannot be
  reproduced or bc-version-pinned on any currently supported version.
- table-relation-test-exclude-known-invalid-relations-via-event.md: added
  the [Scope('OnPrem')] boundary verified against BCApps' Table Relation
  Test codeunit.
- use-assert-isfalse-not-asserterror-for-boolean-checks.md: added a Scope
  section resolving the overlap with asserterror-needs-expectederror-and-code.
- al-testing-review.md: fixed the shared-fixture cue to catch the actual
  anti-pattern instead of the compliant shape, added the missing cue for
  use-assert-isfalse-not-asserterror-for-boolean-checks, wired precedence
  between it and the generic asserterror rule, and removed the cue for the
  deleted article.
- Added in-file Source provenance (specific fluxxus.nl post per article,
  with what was independently verified vs. taken from the post) to the
  three surviving externally-inspired articles, per Jesper's request that
  provenance live in the knowledge file itself, not only the PR description.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-21 22:49:17 +02:00
Michael Dieringer
dcd79afc08 Address Jesper Schulz-Wedde's review on PR #159
- transactionmodel-attribute-governs-test-transactions.md: the "Commit
  causes an error" behavior is specific to an explicitly declared
  AutoRollback attribute. A test method with no TransactionModel
  attribute at all is a distinct, valid shape — BCApps' own
  codeunit 134915 "ERM Online Mapping Setup" commits inside a lazy
  Initialize() with no attribute declared, cleaning up via a manual
  asserterror at the end. Evidence for commit-shared-test-fixture-
  inside-lazy-initialize.md (this PR), which is correct as submitted.
- confirm-needs-strsubstno-before-confirmhandler-sees-substituted-text.md:
  reframe as a known, unconfirmed-fix platform defect
  (microsoft/ALAppExtensions#23935) rather than designed behavior; add
  the Message/MessageHandler asymmetry as supporting evidence.
- table-relation-test-exclude-known-invalid-relations-via-event.md:
  note the test-app-only consumer dependency; correct "walks every
  TableRelation field property in the app" to the actual tenant-wide
  Table Relations Metadata scope across installed apps.
- Wire confirm-needs-strsubstno, commit-shared-test-fixture-inside-
  lazy-initialize, and table-relation-test-exclude-known-invalid-
  relations-via-event into al-testing-review.md's candidate-selection
  cues.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-21 22:48:58 +02:00
Michael Dieringer
12b7f73d24 Add 4 more AL/BC testing patterns from Luc van Vugt's fluxxus.nl blog
Fourth batch from CURABIS ApS, mined from an external BC/NAV testing expert's blog archive (fluxxus.nl). Confirm+StrSubstNo interaction with ConfirmHandler, Table Relation Test's OnAfterRemoveTableRelation exclusion hook (verified against BCApps source, codeunit 134926), committing shared lazy-Initialize fixture data, and Assert.IsFalse vs asserterror for boolean checks.
2026-09-21 22:48:57 +02:00