69b09db3a6
- 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.
2.7 KiB
| bc-version | domain | keywords | technologies | countries | application-area | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
testing |
|
|
|
|
Use Assert.IsFalse to check a boolean result, not asserterror around Assert.IsTrue
Description
asserterror exists to assert that a statement raises a runtime error; it is not a general-purpose way to invert a boolean check. Wrapping asserterror Assert.IsTrue(SomeFunc(), Msg) to verify that SomeFunc() returns false tests whether Assert.IsTrue's own error-raising behavior fired, not the value SomeFunc() actually returned.
Best Practice
When the code under test returns a Boolean rather than raising an error, assert the value directly with Assert.IsFalse(SomeFunc(), Msg) (or Assert.IsTrue for the positive case). Reserve asserterror for statements expected to actually raise an error.
See sample: use-assert-isfalse-not-asserterror-for-boolean-checks.good.al.
Anti Pattern
asserterror Assert.IsTrue(SomeFunc(), Msg); to verify SomeFunc() is false. It passes today because Assert.IsTrue happens to raise an error on failure, but it verifies the assertion helper's error-raising behavior, not the value under test.
See sample: use-assert-isfalse-not-asserterror-for-boolean-checks.bad.al.
Source
Drawn from Luc van Vugt's "TDD in NAV – ASSERTERROR or IsFalse": https://www.fluxxus.nl/index.php/bc/tdd-in-nav-asserterror-or-isfalse/. The post's own example and reasoning — reserve asserterror for the product code actually raising an error, use Assert.IsFalse/Assert.IsTrue to check a boolean the test framework itself computes — carries over directly; the overlap with asserterror-needs-expectederror-and-code.md below is this repository's own addition, not from the source.
Scope
This rule and asserterror-needs-expectederror-and-code.md can both match asserterror Assert.IsTrue(SomeFunc(), Msg); with nothing after it — the generic rule sees a bare asserterror, this one sees asserterror wrapping an Assert.IsTrue/Assert.IsFalse call used to invert a boolean. This rule wins for that shape: the fix is to replace the construct with a direct Assert.IsFalse/Assert.IsTrue call, not to add Assert.ExpectedError/Assert.ExpectedErrorCode after it. asserterror-needs-expectederror-and-code.md still applies on its own to every other bare asserterror, including one guarding Assert.IsTrue/Assert.IsFalse where the intent genuinely is to assert that the guarded call itself raises an error (for example, asserting that a validation helper errors before it can even return a boolean).