Skip to content

[Bug 647094] Re-enable the composite layout resolution tests on a real body layout - #10983

Merged
Morten Rasmussen (v-mortenr) merged 1 commit into
mainfrom
bug/647094
Sep 4, 2026
Merged

[Bug 647094] Re-enable the composite layout resolution tests on a real body layout#10983
Morten Rasmussen (v-mortenr) merged 1 commit into
mainfrom
bug/647094

Conversation

@v-mortenr

@v-mortenr Morten Rasmussen (v-mortenr) commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

What & why

Five test methods in codeunit 134619 Composite Layout Tests were disabled to take up a platform
build that changed how composite layout parts resolve. They asserted a call shape the product never
makes: each inserted a Tenant Report Layout Cfg row keyed by the plain layout name 'Body' on the
fake report 50000, then resolved with that same plain name. Every product caller resolves with
CompositeLayoutKey(Rec) — the <appid>::<name> composite reference — so once the configuration
rows were keyed that way the tests broke and stopped telling us anything about the real resolution
path.

Each of the five now creates an actual body layout on Report::TestReportLayoutsReport and keys its
configuration row by that layout's composite reference, which is what ReportLayoutFactBox and the
Layout Theme and Header/Footer page pass in. The assertions and the precedence levels they cover
are unchanged. Their entries are removed from Tests-Report.DisabledTest.json.

The Initialize() cleanup already deleted the rows on the body-layout report, but its comment
described a leak that can no longer happen, so it now names the report-level rows the assignment
tests actually leave behind.

Linked work

AB#647094 — tracked in ADO (Dynamics SMB); this is an internal test fix with no public issue.

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

  • Built and published Base Application and Tests-Report from a local 29 enlistment both published clean with no new analyzer warnings in the build logs.
  • Ran codeunit 134619 in full (not just the five methods, to catch anything the fixture change
    disturbed): 32 total, 32 passed, 0 failed, 0 skipped,All five formerly-disabled
    methods ran and passed.
  • No product code is touched, so there is nothing to exercise in the UI beyond the test run.

Risk & compatibility

Test-only change — no product code, no schema, no permissions, no telemetry. The change needs a
backport to releases/29.x once merged, since that is where the tests are disabled today; the
matching App/DisabledTests/CompositeLayoutTests.DisabledTest.json removal on the NAV side goes
with that ingestion.

…l body layout

The five Composite Layout Tests methods disabled by PR 252675 asserted a call
shape the product never makes: they inserted a Tenant Report Layout Cfg row
keyed by the plain layout name 'Body' on the fake report 50000, then resolved
with that same plain name. Every product caller resolves with
CompositeLayoutKey(Rec), the <appid>::<name> composite reference, so the tests
broke as soon as the config rows were keyed that way and told us nothing about
the real resolution path.

Each of the five now creates an actual body layout on
Report::TestReportLayoutsReport and keys its configuration row by that layout's
composite reference, which is what the FactBox and the Layout Theme and
Header/Footer page pass in. The assertions and the precedence levels they cover
are unchanged.

The Initialize cleanup already cleared the rows on the body-layout report; its
comment described a leak that can no longer occur, so it now names the
report-level rows the assignment tests actually leave behind.

Codeunit 134619 runs 32/32 green with none skipped.
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept

What this PR does

This change restores five Composite Layout Tests by creating real body layouts and using the composite layout key that product callers use. The setup now matches the actual resolver contract, the assertions still cover the same precedence behavior, and the disabled-test entries are removed while the test methods remain present.

Problem-solution fit

Fit: Strong

The bug is about tests that no longer matched the current composite layout resolver behavior. The diff directly updates those tests to use the real body-layout key shape and re-enables them without changing product behavior.

Suggestions

None.

Risk assessment and necessity

Risk: This is a test-only change in CompositeLayoutTests.Codeunit.al plus disabled-test metadata. There is no product code, schema, permission, public API, event publisher, or cross-repository dependency change; the main regression surface is the shared legacy test fixture, and the existing cleanup still removes the report and layout rows the suite can create.

Necessity: The change is required to make the tests exercise the current real resolution path again. The scope is narrow: it updates only the broken test setup and removes exactly those tests from the disabled list.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10983 round=1 by=alexei-dobriansky at=2026-09-04T08:48:04Z lastSha=9320fe9bbdfaeabad924f64e638a630a3b9e03ae reviewKey=9b9c50ea474669d98928bc1b80de608a968e0fb78169a3b18b8ae85d79429196 suggestions=none

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The restored tests now use real tenant body layouts and the same composite layout key passed by production callers. Their assertions continue to cover the intended precedence, independent part resolution, decoded display, and usage-count behavior, while the shared legacy fixture cleanup covers the new rows and layouts. No blocking findings.

@v-mortenr
Morten Rasmussen (v-mortenr) added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 9757a74 Sep 4, 2026
175 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team: Integrations GitHub request for Integrations area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants