mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Skaerp (Edison): ny defekt-saa-fix-regressionstest-undtagelse
This commit is contained in:
Michael Dieringer
parent
582c6f5f1a
commit
db3380711b
1 changed files with 127 additions and 100 deletions
|
|
@ -1,100 +1,127 @@
|
||||||
---
|
---
|
||||||
bc-version: [all]
|
bc-version: [all]
|
||||||
domain: testing
|
domain: testing
|
||||||
keywords: [test, when, scenario, single-action, bdd, atdd, given-when-then]
|
keywords: [test, when, scenario, single-action, bdd, atdd, given-when-then]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
countries: [w1]
|
countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Each test procedure must contain exactly **one** `[WHEN]` block — one action that
|
Each test procedure must contain exactly **one** `[WHEN]` block — one action that
|
||||||
triggers the behaviour under test. A test with multiple WHENs ("do A, then do B,
|
triggers the behaviour under test. A test with multiple WHENs ("do A, then do B,
|
||||||
then check C") is really two or more tests in disguise. Split them.
|
then check C") is really two or more tests in disguise. Split them.
|
||||||
|
|
||||||
This constraint serves two purposes:
|
This constraint serves two purposes:
|
||||||
|
|
||||||
1. **Failure isolation** — when the test fails you know which action caused it.
|
1. **Failure isolation** — when the test fails you know which action caused it.
|
||||||
2. **Readable specification** — each test reads as a single, falsifiable claim
|
2. **Readable specification** — each test reads as a single, falsifiable claim
|
||||||
about the system's behaviour.
|
about the system's behaviour.
|
||||||
|
|
||||||
A scenario that genuinely requires a precondition action (e.g. "post an order so
|
A scenario that genuinely requires a precondition action (e.g. "post an order so
|
||||||
that a ledger entry exists") belongs in `[GIVEN]`. Only the action being asserted
|
that a ledger entry exists") belongs in `[GIVEN]`. Only the action being asserted
|
||||||
belongs in `[WHEN]`.
|
belongs in `[WHEN]`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
// WRONG: two actions in one test
|
// WRONG: two actions in one test
|
||||||
[Test]
|
[Test]
|
||||||
procedure GetPrice_ThenGetDiscount_ReturnsCorrectValues()
|
procedure GetPrice_ThenGetDiscount_ReturnsCorrectValues()
|
||||||
var
|
var
|
||||||
UnitPrice, LineDiscPct: Decimal;
|
UnitPrice, LineDiscPct: Decimal;
|
||||||
begin
|
begin
|
||||||
// [GIVEN] ...
|
// [GIVEN] ...
|
||||||
// [WHEN] first action
|
// [WHEN] first action
|
||||||
FindPriceMgt.GetSalesPrice(CustomerNo, ItemNo, '', UnitPrice, LineDiscPct);
|
FindPriceMgt.GetSalesPrice(CustomerNo, ItemNo, '', UnitPrice, LineDiscPct);
|
||||||
// [WHEN] second action — this is a second test in disguise
|
// [WHEN] second action — this is a second test in disguise
|
||||||
FindPriceMgt.GetSalesPriceTiers(CustomerNo, ItemNo, '', TempBuffer);
|
FindPriceMgt.GetSalesPriceTiers(CustomerNo, ItemNo, '', TempBuffer);
|
||||||
// [THEN] asserting two unrelated things
|
// [THEN] asserting two unrelated things
|
||||||
Assert.AreEqual(100, UnitPrice, '');
|
Assert.AreEqual(100, UnitPrice, '');
|
||||||
Assert.IsFalse(TempBuffer.IsEmpty(), '');
|
Assert.IsFalse(TempBuffer.IsEmpty(), '');
|
||||||
end;
|
end;
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
// CORRECT: split into two focused tests
|
// CORRECT: split into two focused tests
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
procedure GetPrice_CustomerPrice_ReturnsCorrectUnitPrice()
|
procedure GetPrice_CustomerPrice_ReturnsCorrectUnitPrice()
|
||||||
var
|
var
|
||||||
UnitPrice, LineDiscPct: Decimal;
|
UnitPrice, LineDiscPct: Decimal;
|
||||||
begin
|
begin
|
||||||
// [GIVEN] a customer with a price list line at 100
|
// [GIVEN] a customer with a price list line at 100
|
||||||
WarecoLib.GivenCustomerWithPrice(Customer, Item, '', 100);
|
WarecoLib.GivenCustomerWithPrice(Customer, Item, '', 100);
|
||||||
// [WHEN]
|
// [WHEN]
|
||||||
FindPriceMgt.GetSalesPrice(Customer."No.", Item."No.", '', UnitPrice, LineDiscPct);
|
FindPriceMgt.GetSalesPrice(Customer."No.", Item."No.", '', UnitPrice, LineDiscPct);
|
||||||
// [THEN]
|
// [THEN]
|
||||||
Assert.AreEqual(100, UnitPrice, 'Unit price must match price list');
|
Assert.AreEqual(100, UnitPrice, 'Unit price must match price list');
|
||||||
end;
|
end;
|
||||||
|
|
||||||
[Test]
|
[Test]
|
||||||
procedure GetPriceTiers_CustomerTier_ReturnsOneTierLine()
|
procedure GetPriceTiers_CustomerTier_ReturnsOneTierLine()
|
||||||
var
|
var
|
||||||
TempBuffer: Record "Find Price Tier Buffer" temporary;
|
TempBuffer: Record "Find Price Tier Buffer" temporary;
|
||||||
begin
|
begin
|
||||||
// [GIVEN] a customer with a tier price at min qty 10
|
// [GIVEN] a customer with a tier price at min qty 10
|
||||||
WarecoLib.GivenCustomerWithTierPrice(Customer, Item, '', 10, 90);
|
WarecoLib.GivenCustomerWithTierPrice(Customer, Item, '', 10, 90);
|
||||||
// [WHEN]
|
// [WHEN]
|
||||||
FindPriceMgt.GetSalesPriceTiers(Customer."No.", Item."No.", '', TempBuffer);
|
FindPriceMgt.GetSalesPriceTiers(Customer."No.", Item."No.", '', TempBuffer);
|
||||||
// [THEN]
|
// [THEN]
|
||||||
Assert.AreEqual(1, TempBuffer.Count(), 'Exactly one tier line expected');
|
Assert.AreEqual(1, TempBuffer.Count(), 'Exactly one tier line expected');
|
||||||
end;
|
end;
|
||||||
|
|
||||||
## Flow tests — the deliberate exception
|
## Flow tests — the deliberate exception
|
||||||
|
|
||||||
A **flow test** verifies the accumulated outcome of a multi-round business
|
A **flow test** verifies the accumulated outcome of a multi-round business
|
||||||
flow (partial receipt then invoicing, multiple posting rounds against one
|
flow (partial receipt then invoicing, multiple posting rounds against one
|
||||||
document). The sequence IS the scenario — splitting it loses the interaction
|
document). The sequence IS the scenario — splitting it loses the interaction
|
||||||
under test. Multiple `[WHEN]` blocks are permitted when ALL of these hold:
|
under test. Multiple `[WHEN]` blocks are permitted when ALL of these hold:
|
||||||
|
|
||||||
1. The name declares the flow (`SVPartialFlowTests`,
|
1. The name declares the flow (`SVPartialFlowTests`,
|
||||||
`ReceiveThenInvoice_QuantitiesAreCorrect`).
|
`ReceiveThenInvoice_QuantitiesAreCorrect`).
|
||||||
2. Each `[WHEN]` is labelled as a round of ONE scenario ("Runde 1: kun
|
2. Each `[WHEN]` is labelled as a round of ONE scenario ("Runde 1: kun
|
||||||
modtagelse"), not as an unrelated action.
|
modtagelse"), not as an unrelated action.
|
||||||
3. The `[THEN]` asserts the accumulated end-state. If the assertions
|
3. The `[THEN]` asserts the accumulated end-state. If the assertions
|
||||||
decompose cleanly per action, it is two tests in disguise: split.
|
decompose cleanly per action, it is two tests in disguise: split.
|
||||||
|
|
||||||
Unit-level tests keep the strict one-WHEN rule without exception. (Edison
|
Unit-level tests keep the strict one-WHEN rule without exception. (Edison
|
||||||
eval 2026-07-02, Jernpladsen @ b7656b1: five deliberate round-labelled flow
|
eval 2026-07-02, Jernpladsen @ b7656b1: five deliberate round-labelled flow
|
||||||
procedures in SVPartialFlowTests — the rule previously gave no verdict.)
|
procedures in SVPartialFlowTests — the rule previously gave no verdict.)
|
||||||
|
|
||||||
## Naming implication
|
## Defect-then-fix regression tests — a second, narrower exception
|
||||||
|
|
||||||
The procedure name should make the single WHEN self-evident.
|
A test that reproduces a specific stale/broken state and then verifies a
|
||||||
A name with "And" or "Then" in the middle is a strong signal to split:
|
subsequent action corrects it (`RecalcRestoresStaleDiscountAfterPick`: pick
|
||||||
|
creates the stale state, recalc is the fix under test) is not the same
|
||||||
- `GetPrice_AndDiscount_ReturnsValues` → split
|
shape as an unrelated-action test, even though its `[THEN]` assertions
|
||||||
- `GetPrice_CustomerPrice_ReturnsUnitPrice` → good
|
decompose cleanly per step — decomposing cleanly is expected here, not a
|
||||||
- `ReceiveThenInvoice_QuantitiesAreCorrect` → legitimate flow test IF the
|
sign of two unrelated tests. The three flow-test conditions above are the
|
||||||
flow-test conditions above are met
|
wrong fit for this case: the name doesn't need to declare a multi-round
|
||||||
|
"flow," and there is no natural "Runde 1/2" framing for "create the broken
|
||||||
|
state, then fix it." This shape is permitted when:
|
||||||
|
|
||||||
|
1. The second `[WHEN]` cannot be meaningfully tested without the first —
|
||||||
|
the fix being verified only has an effect on the specific stale state
|
||||||
|
the first action produced, so splitting would require re-running the
|
||||||
|
first action inside a second test's `[GIVEN]` anyway, testing nothing
|
||||||
|
new.
|
||||||
|
2. The procedure name communicates the before/after relationship (a
|
||||||
|
defect symptom and its correction), even without the word "flow."
|
||||||
|
|
||||||
|
Applying flow-test criterion 3 ("assertions decompose cleanly → split") to
|
||||||
|
this shape would have been a false positive (Edison eval 2026-08-13,
|
||||||
|
Wareco @ a2fc8ff8, `SalesOrderAmountAfterPickTest.RecalcRestoresStaleDiscountAfterPick`)
|
||||||
|
— clean decomposition is exactly what a defect-then-fix test's assertions
|
||||||
|
are supposed to do at each step, not evidence the steps belong in separate
|
||||||
|
tests.
|
||||||
|
|
||||||
|
## Naming implication
|
||||||
|
|
||||||
|
The procedure name should make the single WHEN self-evident.
|
||||||
|
A name with "And" or "Then" in the middle is a strong signal to split:
|
||||||
|
|
||||||
|
- `GetPrice_AndDiscount_ReturnsValues` → split
|
||||||
|
- `GetPrice_CustomerPrice_ReturnsUnitPrice` → good
|
||||||
|
- `ReceiveThenInvoice_QuantitiesAreCorrect` → legitimate flow test IF the
|
||||||
|
flow-test conditions above are met
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue