Skip to content

[Bug][Subscription Billing] 67 Subscription Billing tests are disabled in CI on every localization without a tracked reason - #13072

Open
Miljan Milosavljević (miljance) wants to merge 2 commits into
microsoft:mainfrom
miljance:SBDisabledTestsWithoutReason
Open

Miljan Milosavljević (miljance) wants to merge 2 commits into
microsoft:mainfrom
miljance:SBDisabledTestsWithoutReason

Conversation

@miljance

@miljance Miljan Milosavljević (miljance) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What & why

67 Subscription Billing tests were switched off in CI on every localization through src/DisabledTests/Subscription_Billing_Test/Subscription Billing Test.DisabledTest.json. Skipped tests are not shown in the CI log, so a pull request that broke one of these scenarios still got a green build. 66 of the 67 entries had no bug reference.

This PR deletes the DisabledTests file for the Subscription Billing test app, so all 67 tests run in CI again.

Re-enabling the tests exposed one product regression and a few tests that only passed by accident:

  • Product fix, page 8006 Contract Renewal Selection: [master] Renewal Term field on the Create Contract Renewal Quote page does not accept the entered value such as 12M #10276 introduced a regression. After the user opened the Subscription from a contract line (Service Object Description, Service Object No. or the drill-down), the page kept showing the old buffered Subscription Line values. It now reloads the buffered lines after the Subscription is closed. Only the Renewal Term values the user typed on the page are kept, because the page writes them to the Subscription Lines only on closing. This is done in the new local procedure OpenServiceObjectAndRefreshLines.
  • Test fixes: 5 tests depended on the environment (price calculation setup, or an error that was never asserted) and passed by accident. They now set up what they need and assert the expected error.

Linked work

Fixes #12498

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome

  • Ran everything on a BC 30.0.55558.0 W1 container.
  • All 67 previously disabled tests pass.
  • The full Subscription Billing test suite (806 tests) is green. I ran it through al-mcp, and it is also green in the AL Test Tool run from the client.
  • New tests in codeunit 139692 "Contract Renewal Test" for the page 8006 fix:
    • RenewalTermEnteredOnPageIsRetainedAfterSubscriptionIsOpened: the Renewal Term typed on the page is kept after the Subscription is opened, and the Calculation Base % changed on the Subscription is shown.
    • RenewalTermChangedOnSubscriptionIsShownOnPage: a Renewal Term changed on the Subscription is shown on the page.
    • The previously disabled test for the same regression (TestIfContractRenewalSelectionIsUpdateOnAfterValidateCalcBasePercentage) passes too.
  • Mutation testing of the fix: 3 of 4 mutants were killed. The survivor removes CurrPage.Update(false). A TestPage re-reads the current record after the action, so no test can observe the missing refresh.
  • 5 tests that only passed by accident were made independent of the environment:
    • 139692 Contract Renewal Test:
      • CheckDisallowChangesToContractRenewalQuote now asserts the expected error and covers Sell-to Customer No. through the page. The check only runs for changes made on the page.
      • DisallowSalesQuoteToInvoiceForContractRenewal now asserts the expected error and uses ConfirmHandler. CreateFakeContractRenewalQuote now creates the Sales Subscription Line with Process "Contract Renewal" that makes the quote a Contract Renewal Quote.
    • 139690 Contract Price Proposal Test: CreateVendorGLAccountPriceUpdateProposalRecentItemPrices and CreateCustomerPriceUpdateProposalUsesPriceOfSubscriptionUnitOfMeasure now enable extended price calculation and set the default handler to "Business Central (Version 16.0)".
    • 148157 Service Object Test: ExpectErrorOnChangeEndUserIfServiceObjectIsLinkedToContract now links the Subscription to a contract, uses a customer with its own price group, and asserts the error.

Risk & compatibility

  • CI coverage: The tests now also run on all country builds. I only ran W1 locally, so a localization-specific failure can still show up in CI.
  • Possible IT failure: DeferralsReleaseSucceedsWhenGLAccountHasDefaultDeferralTemplateAndJournalTemplNotMandatory was the only entry with a bug reference (641478, failing on IT). It passes on W1 but may still fail on IT. Please expect this one possibly failing in the IT job.
  • Page 8006 behavior change: After opening a Subscription from the page, the lines now show the current Subscription Line values instead of stale buffered values. Typed Renewal Term values are still kept. There is no data or upgrade impact, and no permission or telemetry change.
  • Scope: The only product change is page 8006. The other changes are tests and the removed DisabledTests file.

🤖 Generated with Claude Code

…sts and refresh Contract Renewal Selection after opening the Subscription

The Subscription Billing DisabledTests list (67 entries, 66 without a bug
reference) is removed, so all Subscription Billing tests run in CI again.

TestIfContractRenewalSelectionIsUpdateOnAfterValidateCalcBasePercentage
failed because of a real regression: since microsoft#10276 the Contract Renewal
Selection page loads a line into its buffer only once. Changes made on the
Subscription opened from the page were not shown, and a Renewal Term changed
there was overwritten with the stale buffer value when the page was closed.
The page now reloads the buffered lines of that Subscription after the card
closes and keeps only the Renewal Terms entered on the page itself.

Tests that only passed by accident are made independent of the environment:
- CheckDisallowChangesToContractRenewalQuote and
  DisallowSalesQuoteToInvoiceForContractRenewal now build a real Contract
  Renewal Quote and assert the expected error.
- ExpectErrorOnChangeEndUserIfServiceObjectIsLinkedToContract asserts the
  expected error for a Subscription linked to a contract.
- The two price update proposal tests set up the price calculation handler
  they depend on.

Fixes microsoft#12498

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 From Fork Pull request is coming from a fork Team: Finance GitHub request for Finance area needs-approval Workflow runs require maintainer approval to start labels Oct 20, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change re-enables the full Subscription Billing test set, makes several tests independent of environment defaults, and fixes stale values on the Contract Renewal Selection page after a Subscription is opened.

The deleted disabled-test list makes the scenarios run again. The page refresh reloads the changed Subscription Lines while preserving Renewal Terms entered on the selection page, and the new tests cover both the refreshed calculation value and the two Renewal Term paths. The refresh is correct for the built-in fields, but it bypasses the extension hooks that can add values to the temporary buffer.

Problem-solution fit

Fit: Strong

The change directly restores the missing CI coverage and addresses the stale page-buffer regression exposed by those tests. The product change is limited to the affected page, while the remaining edits make existing tests assert the intended behavior.

Suggestions

S1 (🔴 High): Preserve extension values during buffer refresh
TransferFields overwrites values that subscribers can add through OnAfterInitTempSubscriptionLine or OnValidateRenewalTermOnBeforeCurrPageUpdate. After the Subscription closes, those extension values can be lost or transferred back incorrectly; refresh through an extension-aware hook or reapply the buffer hooks before updating the page.

Risk assessment and necessity

Risk: Re-enabling the suite can expose localization-specific failures, so the country jobs, especially the known IT-sensitive deferral scenario, should be green before merge. The product change is narrow, but the new full-record refresh can break extensions that populate the temporary Subscription Line buffer through the page's integration events.

Necessity: Restoring these tests is important because skipped scenarios can hide regressions across contract renewal, deferrals, and billing. Refreshing the page after editing a Subscription is also needed so users do not continue with stale line values; preserving the existing extension contract keeps that fix safe.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=13072 round=1 by=alexei-dobriansky at=2026-10-21T01:05:03Z lastSha=ba5d80e53e67a2f5917a064e59349e1440eb471b reviewKey=885ed772e1d2080cf3e0319dd1fe65812dcbeebc5368883584abef1c7c9ceb7a suggestions=S1@75e3f236

… Renewal Selection

After the Subscription is opened from the Contract Renewal Selection page,
the buffered lines were refreshed with TransferFields, which also overwrote
values that subscribers set in the buffer through OnAfterInitTempSubscriptionLine
or OnValidateRenewalTermOnBeforeCurrPageUpdate. Those values were then lost
or transferred back incorrectly on closing.

The refresh now takes a field from the Subscription Line only when the buffer
still held the value stored before the Subscription was opened. Values that
differ only on the page, such as an entered Renewal Term or a value set by a
subscriber, are kept and written on closing as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@miljance

Copy link
Copy Markdown
Contributor Author

Thanks, S1 is addressed in 6f7561a.

The refresh after the Subscription is opened no longer uses TransferFields. A buffer field now takes the current Subscription Line value only if the buffer still held the value the line had before the Subscription was opened. Values that differ only on the page are kept and are written back on closing as before (through TransferChangedValuesFromBufferToServiceCommitment and OnTransferChangedValuesFromBufferToSubscriptionLine). This covers a Renewal Term entered on the page and values set by subscribers of OnAfterInitTempSubscriptionLine or OnValidateRenewalTermOnBeforeCurrPageUpdate. FlowFields, system fields and Blob/Media fields are skipped, and Quantity is recalculated.

New test ValueSetBySubscriberIsRetainedAfterSubscriptionIsOpened in codeunit 139692. The test codeunit binds itself as a manual subscriber to OnAfterInitTempSubscriptionLine and sets Term Until on the page lines. After the Subscription is opened and its Calculation Base % is changed, the page shows the new Calculation Base % and still the Term Until set by the subscriber. The test failed on the previous commit for exactly this reason.

The full Subscription Billing suite is green on W1.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

The new commit replaces the blanket buffer refresh with a field-level three-way merge. Database changes refresh fields that the page has not changed, while page-only and subscriber-provided values remain intact.

Status of previous suggestions
ID Title Status Author response
S1 Preserve extension values during buffer refresh Addressed The refresh now compares the current buffer with a pre-card snapshot, and a subscriber-based regression test verifies that page-only values survive.
New observations (commits since round 1)

None - the new implementation preserves page-only values while refreshing card changes, and the changed paths have focused coverage.

Risk assessment and necessity

Risk: The reflection-based refresh covers normal base and extension fields and excludes FlowFields, system fields, Blob, and Media fields. Quantity is recalculated explicitly, and no new correctness issue was found.

Necessity: The change restores useful CI coverage and prevents stale Contract Renewal Selection values without overwriting extension-provided page values.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=13072 round=2 by=alexei-dobriansky at=2026-10-21T13:29:50.6070401Z lastSha=6f7561a424d1f7620d3b142e181dc5e8702590f8 reviewKey=5a0cc95f16342d3d2a4aecb08e40d457b828809ccad3096823d5ea73acbc7c47 suggestions=S1@75e3f236:addressed parentRound=1

This branch was successfully deployed

1 active (outdated) deployment
triage — ba5d80e5 Deployed Oct 20, 2026 by miljance via Classify team ownership #8460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 From Fork Pull request is coming from a fork needs-approval Workflow runs require maintainer approval to start Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][Subscription Billing] 67 Subscription Billing tests are disabled in CI on every localization without a tracked reason

2 participants