Skip to content

[API tests] Isolate localized document comparison exclusions - #11229

Draft
Prangshuman Das (t-prda) wants to merge 5 commits into
prdas/646383-split-journal-numberingfrom
prdas/646383-split-field-comparison
Draft

Prangshuman Das (t-prda) wants to merge 5 commits into
prdas/646383-split-journal-numberingfrom
prdas/646383-split-field-comparison

Conversation

@t-prda

@t-prda Prangshuman Das (t-prda) commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Scope

Add the dynamic AddFieldToIgnoreIfExists helper and its ten comparison callers. Ignore Operation Occurred Date where present, VAT Reporting Date for sales quotes, and Order Date consistently for the V2 purchase-order comparison. This intentionally changes assertion coverage and needs explicit review.

AB#646383 (link only).

Evidence and limits

No corresponding failure is established by saved result sets 2 or 4. The ten affected comparison methods are source-dependency coverage, not ten demonstrated failures. Validate intended API/UI differences before approving the broader ignored-field list.

Only this layer's original fix/exclusion patch is reviewed here (13 paths); the patch is unchanged by Expense-first restructuring.

Exact head: a8973702891fbf3deb8aedf2f28d2fb8dc64e5e4; tree: 30bf84d58c51ffb152ec055e7bea045e78651ed6.

Expense-first sequencing using existing exclusions

The same 193 method-specific temporary exclusions (135 APIV1 +58 APIV2 across12 codeunits) now live in the existing src/DisabledTests/_Exclude_APIV1__Tests/_Exclude_APIV1__Tests.DisabledTest.json and src/DisabledTests/_Exclude_APIV2__Tests/_Exclude_APIV2__Tests.DisabledTest.json. No separate sequencing files remain. Original entries retain their order/style/content; no new duplicate or wildcard/codeunit-wide exclusion is introduced. Unrelated pre-existing APIV2 duplicates are preserved rather than mixed with this change.

#11860 removes the temporary additions from these same existing manifests alongside its already-reviewed authentication uptake. All193 temporary stage entries are removed in #11860, but it makes 192/193 target methods eligible: the independent 139739::TestDeleteInUse exclusion remains until the VAT fixture fix #11224 removes it. From #11224 onward all193 are eligible; eligibility is not runtime success. All general/downstream cumulative trees are exactly byte-identical to the preceding checkpoint; the full checkpoint has none of these193 methods excluded. No app-name allowlist, selection setting, runner/authentication/test/production change or Logiq exclusion is added.

Why these methods started executing: they already required Disabled isolation and were IntegrationTest codeunits. Ordinary typed selection in TestSuiteMgt332–352 selects None|Codeunit; the old extra Disabled pass in RunTestsInBcContainer was UnitTest-only. The clean-execution switch newly reaches Disabled IntegrationTest codeunits and still honors the existing JSON exclusions. These193 methods were not listed in those exclusions. This is a pre-existing selection gap, not a newly introduced product auth bug or proof they never ran in any historical configuration. A complete67-codeunit audit preserves existing UnitTest/Legacy behavior and the ten independently handled Logiq integration tests.

**Expense scope is unchanged:**51 API reenables, nine non-API exclusions, baseline six cases and all consolidated regressions. The two legacy Spend Requests methods remain conditional on not CLEAN30. Focused #12325 review:21 files, including the two existing exclusion manifests. General #11860 review:159 files.

The official NAV Disable-NAVALTest helper was inspected: it has no destination/app-file parameter, writes NAV's App/DisabledTests using per-codeunit filenames and sorts entries. It cannot safely preserve these BCApps files. A bounded JSON relocation preserved original prefixes/order and checked exact identities, then exercised the unchanged real loader. No new shared helper/framework or NAV selector change was made.

Validation and presentation evidence

Pester117/117 passed independently at the new Expense/general/full heads. Actual existing-loader checks confirm identical effective193-method selection after relocation and preserved51/9 Expense scope. All nine downstream Git tree hashes remain exactly unchanged. Fresh exact-head CI is pending; old-head successes are not substituted for current runtime evidence.

Historical run37330893173 at head88881be reached193 general methods:171 genuine401 failures and22 nominal passes (17 bare-ASSERTERROR cases can accept the wrong error; five local fixtures). Separately, all51 Expense methods passed in13 inspected countries (663 results), and all63 touched API methods yielded819 results; Activity coverage was117/198 at that checkpoint. These are bounded historical results, not a full/current matrix pass. The preceding stage-fix runs hit50 hosted-runner acquisition cancellations; other jobs were active, so this was not a claimed global outage. New pushes schedule fresh CI, not an AL retry. The general SQL-pool NRE and missing warning-reference artifacts remain separate unresolved limitations.

Durable session presentation notes: api-test-enablement-presentation-notes.md, with source-pinned selection proof, fix/coverage inventory, helper limitations and historical-vs-current evidence boundaries. Fresh runtime proof must still establish the51 Expense cases and actual193-case suppression at Expense stage.

AB#646383 (link only). Native stack #12327 remains #12325 → #11860 → #11224 → #11225 → #11226 → #11227 → #11228 → #11229 → #11230 → #11322. Protected merged #10085/#11862/#11891 and validation #11892 are untouched; validation-only #11861/#11455 remain Do Not Merge. No new main integration, native-group/base change, PR merge, queue operation, Actions cancellation or manual retry was made. All prior heads remain ancestors; source publication used backups and an atomic forward-only push with explicit leases.

Exact current head: a8973702891fbf3deb8aedf2f28d2fb8dc64e5e4; source tree: 30bf84d58c51ffb152ec055e7bea045e78651ed6.

@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Team: Integrations GitHub request for Integrations area labels Sep 8, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 8, 2026
@t-prda
Prangshuman Das (t-prda) changed the base branch from main to prdas/646383-split-journal-numbering September 8, 2026 17:06
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

@t-prda
Prangshuman Das (t-prda) removed this pull request from stack #11232 September 24, 2026 15:27
@t-prda
Prangshuman Das (t-prda) added this pull request to stack #11863 September 24, 2026 15:27
@t-prda
Prangshuman Das (t-prda) removed this pull request from stack #11863 September 25, 2026 08:46
@t-prda
Prangshuman Das (t-prda) added this pull request to stack #11893 September 25, 2026 08:47
@github-actions

Copy link
Copy Markdown
Contributor

Issue #11561 is not valid. Please make sure you link an issue that exists, is open and is approved.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept with Suggestions

What this PR does

This change adds a name-based helper for optional table fields, uses it in ten API and page record comparisons, and re-enables those tests. The helper safely skips fields that are absent, but the new exclusions weaken some assertions beyond the expected differences currently established.

Problem-solution fit

Fit: Partial

The change supports running the comparisons across localizations, but the unconditional date exclusions can also hide differences that the tests are intended to detect.

Suggestions

S1 (🟠 Moderate): Keep Order Date comparison outside midnight
Both creation paths set Order Date to the same WorkDate(), and the existing comment limits the known time-zone difference to the near-midnight window. Keep this exclusion conditional so the test still detects API and page Order Date regressions during normal runs.

S2 (🟠 Moderate): Keep localized VAT dates in parity checks
The new ignores remove localized date fields from comparisons although both paths initialize them from the same work, document, or posting dates. Ignore them only under a proven localization or time-zone difference; otherwise these tests can pass with a wrong VAT-relevant date.

Risk assessment and necessity

Risk: Production behavior is unchanged, but the test oracle is weakened for localized document dates and the purchase-order Order Date. Future API defaulting or mapping regressions could pass unnoticed.

Necessity: Conditionally handling fields that exist only in some localizations is useful for restoring the disabled suites. The unconditional exclusions are not yet shown to be necessary.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11229 round=1 by=alexei-dobriansky at=2026-10-02T00:05:17+02:00 lastSha=4b7fc4febb6b8d95ee78dde70db1d8f5d5466c48 reviewKey=26ba8a1a6fc0ad973b3a594f37a742eac982ef19538e8cfaf1afe0310255d992 suggestions=S1@784bad11,S2@0845de38

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Issue #11224 is not valid. Please make sure you link an issue that exists, is open and is approved.

This branch was previously deployed

1 inactive (outdated) deployment
triage — 2241ac4d Deployed Sep 8, 2026 by t-prda via Classify team ownership #4286
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 Team: Integrations GitHub request for Integrations area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants