fix(integration-tests): Refactor support claims and bundles to have cleaner logic flow - #11494
Open
adickin-amd wants to merge 15 commits into
Open
fix(integration-tests): Refactor support claims and bundles to have cleaner logic flow#11494adickin-amd wants to merge 15 commits into
adickin-amd wants to merge 15 commits into
Conversation
JIRA ID: ALMIOPEN-2333 --enforce-support-claims only enforced a claim when that specific graph happened to reach the engine query, which sat inside executeGraphThroughEngine. Every early return in runComparison() skipped it, and the run-level guard only fires when *no* graph anywhere was queried, so one healthy bundle masked the rest. Reproduced on gfx1151/Windows: a bundle whose sidecar claims MIOPEN_ENGINE for a graph MIOpen declines FAILs under --verification-mode=auto and exits 0 under --verification-mode=golden, with the broken claim reported under neither satisfied nor broken. Hoist the query - Adjudicate claims at the top of TestBody(), above runComparison(), behind a virtual observeSupportForBundle() seam so the deviceless harnesses still run. - Add a per-graph coverage invariant: a sidecar that exists but was never queried fails that test, so a partial gap is loud immediately rather than surviving behind one healthy bundle. - Reuse the ranked list in the executor and the enforcement rungs, so one test makes one heuristic query instead of two. One engine per lane - observeSupport() takes the single engine under test rather than every loaded engine. Claims naming other engines belong to those engines' lanes; the static inventory covers engines with no lane. - Drop ENGINE_NOT_LOADED: main() already exits non-zero when --test-engine names an engine that is not loaded. - Inject the engine at registration instead of reaching into TestConfig and LoadedEngineTable from inside the test body. - Require --test-engine for enforcement, uniformly. FULL previously enforced without one while non-FULL did not, which left non-FULL claims unenforced. Accepted is not confirmed - Split SATISFIED into CLAIM_ACCEPTED (advertised, from the query) and CLAIM_CONFIRMED / CLAIM_FAILED_IN_USE (promoted once the engine has actually run the graph). Previously a claim was reported satisfied on a graph the same run proved the engine computes wrong, which would feed a support matrix a cell that does not work. - Verdicts are held and published from exactly one place, so the terminal failure path cannot drop rows from the report. - Promotion keys on whether executeGraphThroughEngine actually ran, not on skip state. The APPLICABILITY and BUILDABLE rungs never execute, so they now report accepted rather than confirmed. Report honesty - Drop the neither-claimed-nor-supported quadrant; it carried no information and was half the records. - Separate sidecarChecked (coverage) from the verdict vector, so a sidecar that claims another arch, platform, or sweep case still counts as covered. - Name the coverage shortfall as filter attribution instead of leaving a bare mismatch, and count graphs whose sidecar promises nothing for this cell. Golden means golden - --verification-mode=golden and the former golden-check now FAIL when a bundle has no golden data. An explicit mode is a demand for a specific oracle; auto is the mode with a fallback chain. Separate golden-data validation from engine verification - Add BundleReferenceValidationHarness: recomputes outputs with a reference and compares against the checked-in golden data. No engine, no claims. - Add ReferenceOpCoverage: a per-reference required-op set. Registration only creates a test when the bundle has golden data and every node type is in that reference's set, so the harness has no skip path — an inapplicable reference is a gap in the reference, not a property of the bundle. Bundles outside a set are absent from the suite and the counts are printed at registration. - Replace --verification-mode=golden-check with --validate-golden-data cpu|gpu; the old spelling is a hard error naming its replacement. Tests - New TestSupportClaimEnforcement drives the real TestBody() deviceless and pins the closed gaps: golden without data still queries claims, a broken claim is terminal before the engine, and the promotion mapping. - New TestReferenceOpCoverage pins the op sets and the unreadable-graph case. - TestSupportVerdict rewritten against on-disk sidecars, covering the sweep dispatch and arch/platform scoping that had no coverage before. - Add TestConfig::isInitialized() so harness-driving tests need not guess at suite ordering. 422 unit tests pass (2 skipped, golden data not pulled). Verified on gfx1151/Windows against both lanes.
…by skip state JIRA ID: ALMIOPEN-2333 TestBody() read like a pipeline but its stages communicated through GTest globals and two mutable members. runComparison() returned void; what it did was reconstructed afterwards from HasFatalFailure(), IsSkipped(), HasFailure(), _verified and _engineRan. _verified was a mutable bool written from five places, one of them a const method. GTEST_SKIP()/FAIL() fired from six different depths, so "what happened" was only recoverable as a gtest verdict, never as a value. That shape produced two defects. Confirmed cells that nothing verified - Promotion keyed on exercised=_engineRan. When the engine executed and the fallback chain then ran out of oracles, skipUnverifiable() left _engineRan true and HasFailure() false, so promoteAcceptedClaim(true, true) published CLAIM_CONFIRMED with detail "graph executed and verified". Nothing had been compared, and a published support matrix reads the confirmed column. - Confirmation is now measured against the bundle's own enforcement_level. A full bundle that executed and found no oracle stays accepted; a buildable bundle whose plans compiled is confirmed, which it never could be before. Oracle failures blamed on the engine - passed=!HasFailure() meant a CPU-reference RUNTIME_ERROR marked the engine's claim CLAIM_FAILED_IN_USE, the "do not publish this cell" signal, for a defect in the reference. Outcomes now carry who broke; only ENGINE and COMPARISON demote a claim. A broken reference or missing golden data still fails the run and leaves the claim where the query put it. One value, one verdict, one disposition - New VerificationOutcome (status, depth, origin, message). VerificationDepth is an ordered ladder shared by the enforcement rungs and the comparison: NOT_REACHED < APPLICABLE < BUILDABLE < EXECUTED < VERIFIED. - Everything under TestBody() returns one instead of skipping or failing where it stands. commitClaims() is the only verdict site; reportOutcome() the only GTEST_SKIP/FAIL. HasFatalFailure() survives at exactly one seam, right after the virtual executor that is allowed to ASSERT. - _verified and _engineRan are deleted; depth subsumes both. The "verified nothing" guard becomes a structural check that a PASSED outcome reached the depth its bundle asks for. - claimBlocked() replaces aggregateClaimFailures(): the gate is an optional<VerificationOutcome> rather than a control-flow decision riding on a string being empty. - SupportObservation::sidecarChecked becomes SidecarState NONE/CHECKED. The field must never be derived from results.empty(), and deriving it is what used to fail healthy runs; naming both states makes that mistake something you have to write on purpose. The golden -> GPU ref -> CPU ref -> skip chain is unchanged: same order, same predicates, same messages, same UnverifiableBundleReport records. Only the plumbing under it returns values. Dedup - buildGraph() replaces three copies of the from_binary block and rankedEngineIds() two copies of the ranked-query memo. Tests - The promotion policy moves into TestSupportVerdict, where it needs no fixture. It could not be driven through TestBody() before: the fake part-result reporter hides HasFailure()/IsSkipped() from the harness. - TestSupportClaimEnforcement gains five cases driven through the real body: mismatch demotes, executed-without-an-oracle stays accepted, reference error does not demote, a buildable bundle confirms at its own depth, an unreached rung stays accepted. 425 unit tests pass (2 skipped, golden data not pulled). Verified end to end on gfx1151/Windows against MIOPEN_ENGINE: 2457 integration tests pass; a planted sweep sidecar yields confirmed 2 / unclaimed 7, and a claim for a graph MIOpen declines fails once with the claim message and no sentinel diff stacked on it.
…rgument JIRA ID: ALMIOPEN-2333 Three places each built the bundle's graph and re-derived whether the engine would take it: observeSupportForBundle() for the claim verdict, executeGraphThroughEngine() to decide whether to throw EngineNotApplicableError, and enforceAtLevel() to decide whether to skip the rung. Two from_binary calls per test, one memoized ranked query hidden behind a mutable _query member, and one predicate spelled three different ways -- observeSupport() treated GRAPH_NOT_SUPPORTED as resolved-and-declined while the other two treated it as query-failed. They agreed by coincidence, not by construction. One graph, passed down - openGraph() is now the only place a graph is built and the only place get_ranked_engine_ids() is called. It returns a GraphSession that TestBody() hands to runComparison(), the mode functions, runEngine() and enforceAtLevel(). No harness state: _query and the RankedQuery struct are gone. - Reusing one Graph for the query and then create_execution_plans() is what the unpinned path has always done -- with enforcement off, the executor queried the same object it planned on -- and get_ranked_engine_ids() only writes its out param. The full MIOpen suite confirms it: 2457 passed, 0 failed, and 462s against 1006s before, since each test now loads the graph once. - enginesAccept() is the single definition of applicability, computed once at the query. The claim verdict, runEngine() and the rungs all read the same bool, so a graph cannot be a broken claim to one and an engine decline to another. - The decline check moves out of the virtual executor and into runEngine(), so production no longer raises EngineNotApplicableError for a fact it already has. The type and the catch stay: a stubbed executor throws it, and a provider may decline later than the ranked list suggested. - from_binary failure is checked once, at the top of runComparison(), instead of three times with three different reactions. Extracted so the decisions are testable without a device - GraphSession.hpp: RankedEngines (status, message, rankedIds, accepted) split from the session, because every decision needs only that part and that part needs no handle, no graph and no device. Plus isResolved() and enginesAccept(). - SupportVerdict: claimBlocked() and finalizeClaims() move out of the harness. Both were pure already, reachable only by driving TestBody() through a fake part-result reporter. - SupportClaimReport: coverageFor() returns a CoverageUpdate instead of poking the process-wide counters inline. - The harness methods left behind are appliers: recordClaimCoverage() bumps what coverageFor() decided, commitClaims() records what finalizeClaims() returned. Tests - New TestGraphSession pins the applicability predicate: pinned engine present, absent, empty list; GRAPH_NOT_SUPPORTED as a decline rather than an unknown; an unresolved query never accepted even when the id is in the list; the unpinned lane; and the default session accepting nothing. - TestSupportVerdict gains direct coverage of claimBlocked (held claims do not block, broken and errored both block as engine failures, every failing verdict reaches the message) and finalizeClaims (only the engine under test is promoted, the depth reaches the detail, drift is annotated, failing verdicts pass through). - TestSupportClaimReport gains coverageFor cases, including the one that matters: a sidecar read in full with no verdicts still counts as queried. - The three harnesses that stub the executor now stub openGraph() too, and say what they simulate by setting engines.accepted. They were reaching a real device through the shared handle to build a graph they never executed; the unit suite drops from 6.5s to 2.3s without it. 450 unit tests pass (2 skipped, golden data not pulled). Verified on gfx1151/Windows: full MIOpen suite 2457 passed / 0 failed, and planted sidecars still yield confirmed 2, unclaimed 7, and one CLAIM_BROKEN that fails its test once with the claim message.
… the comparison
JIRA ID: ALMIOPEN-2333
Three readability problems, each hiding a decision inside something else.
observeSupport() decided by nesting
- The verdict and its detail string were both chosen by ternaries inside a
push_back, nested under two conditions. Reading off what a given scenario
produced meant unpicking `claimed && !resolved` against `accepted ? A : B`
twice over.
- chooseVerdict(claimed, resolved, accepted) is now the whole table, one
early-return per row, returning nullopt for the quadrant that carries no
information. verdictDetail() owns the strings. observeSupport() loads the
claim, asks the table, and builds one result.
- All eight rows are pinned directly, including that unresolved beats acceptance
(a hit in a list we cannot believe is still QUERY_ERRORED) and that neither
claimed nor accepted records nothing.
TestBody() hid the gate in a ternary
- `blocked ? *blocked : runComparison(session)` made the reader work out that a
failed claim stands in for the comparison. runVerification() names it and says
why in one place, and the body reads as three phases with no conditionals.
The comparison could not be reached without a test body
- compareEach()/compareOutputTensor() walked the outputs, resolved tolerances,
formatted diffs and called EXPECT_TRUE from the middle of the loop. The only
way to exercise any of it was to run a whole TestBody() and read the failure
back out of a fake part-result reporter, so none of it had direct coverage.
- New OutputComparison.{hpp,cpp}: compareTensor() returns an optional
TensorMismatch, compareOutputs() returns one per drifted tensor, and
tensorLabel() resolves the name-or-uid label. No gtest, no TestConfig, no
harness state -- tolerance resolution is a callback the harness supplies, so
the TOML override stays where it belongs.
- The harness keeps compareAgainst(), which resolves tolerances, calls the
comparison and turns each mismatch into one ADD_FAILURE. Behaviour is
unchanged: still one failure per tensor, still every tensor compared after the
first mismatch, still no message on the outcome because the diffs are already
on the record.
- OutputComparison.hpp now owns the OutputTensors alias, so the harness header
stops declaring its own copy.
Tests
- TestOutputComparison drives the comparison directly for the first time: label
fallback, match and mismatch, that the report names the bundle/tensor/values,
that tolerance decides whether a difference matters, that every drifted tensor
is reported rather than just the first, and that only the requested uids are
compared.
- TestSupportVerdict gains the eight-row chooseVerdict table plus verdictDetail
coverage (every verdict explains itself; the two failing ones name the status).
466 unit tests pass (2 skipped, golden data not pulled), up from 450. Full MIOpen
suite on gfx1151/Windows unchanged: 2457 passed, 0 failed.
…laims JIRA ID: ALMIOPEN-2333 "Adjudicate" was doing no work that "check" does not, and it read as jargon in a harness whose other names are plain. The seam is now checkSupportClaims(), which also lines up with SidecarState::CHECKED: calling it is exactly what moves a sidecar from NONE to CHECKED. Deliberately not checkEarlySupportClaimFailures. The function returns the whole SupportObservation, and failures are its rare output -- most runs yield CLAIM_ACCEPTED, UNCLAIMED_SUPPORT, or nothing at all. Its `sidecar` field also feeds the coverage counters, which have nothing to do with failures, and both later phases consume its result. The function that does check for an early failure is claimBlocked(), which already carries that name and that job. Also drops the remaining "adjudicate" prose across the harness, main.cpp, the README and the enforcement doc, so the vocabulary is consistent: a sidecar is read and checked, a verdict is decided, a claim is confirmed. 494 unit tests run, 492 pass (2 skipped, golden data not pulled).
JIRA ID: ALMIOPEN-2333 runVerification() named the decision but hid it: reading TestBody() told you a verification happened, not that there are two ways a test can reach its verdict. The branch is now written out, so the body says what it does -- a failed claim supplies the outcome directly, anything else runs the comparison. commitClaims() and reportOutcome() stay on one shared tail rather than being duplicated into an early return. Exactly one verdict site and exactly one pass/fail/skip site is the property that keeps a path from silently forgetting to publish, and it is worth more than the two lines an early return would save. The "did the test actually reach the depth its bundle asks for" check moves inside the comparison branch, where it is the only place it can say anything: a blocked claim is already a failure at NOT_REACHED, so asking it there was always a no-op. 494 unit tests run, 492 pass (2 skipped, golden data not pulled).
JIRA ID: ALMIOPEN-2333
IntegrationBundleVerificationHarness had nine protected virtuals that existed
only so tests could subclass it, and four suites did exactly that -- each
re-stubbing an overlapping subset. Testing the harness meant reimplementing it.
The nine virtuals are now four injected collaborators and one policy value:
IGraphEngineRunner from_binary, the ranked query, plan compilation and
execution. One seam, not three, because all of them
need the same shared handle and the same device.
IReferenceExecutors the run's reference executors, borrowed rather than
built fresh for every bundle.
ISupportClaimObserver one graph's claim verdicts.
IVerificationReporter the three process-wide report sinks.
HarnessPolicy mode, enforcement, arch, platform, VRAM, and where the
variant pack's memory lives.
The class now has no virtual members and no subclasses. Tests construct the real
harness with gmock doubles, following the mocks/MockX.hpp convention the three
providers already use.
enforceAtLevel() is the point of the exercise. Its BUILDABLE rung compiled plans
against a real graph, so every test stubbed the whole method out and could only
assert on routing. It is now ordinary decision logic over one injected call, and
TestEnforcementRungs.cpp covers both rungs directly for the first time.
Two behaviour changes fall out of returning values instead of asserting:
- runEngine() no longer reads ::testing::Test::HasFatalFailure() to decide the
engine broke. That could not tell an engine assertion apart from any other
fatal failure in the same test, and left the outcome message empty. The
runner returns an EngineOpResult, so the failure carries the frontend's own
text and is attributed to the engine only when the engine is at fault.
- A provider that declines while compiling plans used to escape enforceAtLevel()
as an uncaught exception. It is now an unverifiable skip: the same answer as
declining earlier, just arrived at later.
BundleReferenceValidationHarness takes the same executor container, which drops
its own duplicate factory virtual. The container is process-wide, so the GPU
plan-builder registry is built once per run rather than once per bundle.
The unit-test target deliberately does not compile FrontendGraphEngineRunner.cpp,
ReferenceExecutorPool.cpp or HarnessDependencies.cpp. That makes "these tests
never reach an engine or a device" a link-time guarantee rather than a
convention.
The suites also lose four copies of the same bundle-writing fixture and four of
the same result-inspection helpers, now shared in tests/BundleFixtureFiles.hpp
and tests/HarnessTestSupport.hpp.
504 unit tests run, 502 pass (2 skipped, golden data not pulled), up from 494/492
with no test dropped. miopen-provider external integration on gfx1151: 2562
passed, 1 failed -- quick_ConvolutionWrw_Default.1d_filter3_pad1_stride2_fp32_
ncl_pad1_stride2 crashes with the same access violation on the unmodified parent
commit, so it is pre-existing.
…ndows
JIRA ID: ALMIOPEN-2333
The format targets shelled out to `find . ... -exec ${CLANG_FORMAT_BINARY} ...`.
On Windows cmd resolves `find` to C:\Windows\System32\find.exe, which is a string
search tool, so hipdnn-integration-tests-format failed with "File not found -
*.cpp" and no file was ever formatted.
Separately, findAndCheckTool() unsets CLANG_FORMAT_BINARY on a version mismatch
and returns, but ClangCheck.cmake created the targets regardless. The empty
variable expanded away and left `-exec --verbose -i {} +`, so a clang-format the
build had just declared unusable still produced a reachable target that died with
an unrelated-looking error. That half is not Windows-specific: any host whose
clang-format is not the pinned version hits it.
Both are fixed by adopting what projects/hipdnn already does -- a Python driver
that walks the tree itself, chunked and run in parallel -- plus a guard so an
unusable clang-format creates no targets at all instead of broken ones.
Two deliberate differences from the hipdnn copy:
- src/third-party/ is excluded. The old find only pruned ./build, so it would
have reformatted the vendored argparse.hpp and toml.hpp had it ever run.
- The RunClangFormat.py path is captured before the function that uses it.
CMAKE_CURRENT_LIST_DIR is evaluated where a function runs, not where it is
defined, and only happens to be correct today because the call site is in
the same file.
The version pin is left alone. clang-format 18 and 20 disagree on output, so
raising EXPECTED_CLANG_FORMAT_VERSION here would reformat the tree away from CI.
Hosts running a newer clang-format can pass -DALLOW_CLANG_FORMAT_VERSION_MISMATCH=ON.
Verified on Windows: check-format and format both run for integration-tests and
miopen-provider, the check fails on deliberately malformed input and passes on
the tree, write mode leaves the worktree byte-identical, and a version mismatch
reports "Skipped creating format targets" and creates none.
adickin-amd
marked this pull request as ready for review
August 31, 2026 16:32
adickin-amd
changed the title
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
adickin-amd
commented
Aug 31, 2026
JIRA ID: ALMIOPEN-2333
The Linux superbuild runs clang-tidy with warnings-as-errors and the Windows
superbuild runs the test-name validator. Neither gate is reachable from a local
Windows build -- ClangCheck.cmake reports "Skipped creating 'tidy' targets; not
available on Windows" -- so both only surfaced once this branch reached CI.
clang-tidy:
- Drops three unused using-declarations (RankedEngines, isResolved,
TensorMismatch) left behind when those tests were split up.
- Makes reportOutcome() static. It switches on the outcome and touches no
member state, which is the property that makes the disposition a pure
function of the value the pipeline returned.
- Renames the function-local static in sharedReferenceExecutors() to s_pool,
per readability-identifier-naming.StaticVariablePrefix.
- Moves the SupportObservation into the gmock Return() action instead of
copying it once.
Test-name validator: a test *case* name may not contain the positional keywords
that belong in the *suite* name -- Test, Integration, Gpu, or a datatype token.
Six cases violated that, so they are renamed without changing what they assert:
CpuAndGpuSetsAreNonEmpty -> BothReferenceSetsAreNonEmpty
GpuDoesNotCoverBatchnormInference -> DeviceReferenceDoesNotCoverBatchnormInference
UnqueriedSidecarFailsTheTest -> UnqueriedSidecarFailsTheRun
AcceptedBecomesConfirmedWhenTheTestPasses
-> AcceptedBecomesConfirmedWhenTheRunPasses
OnlyTheEngineUnderTestIsPromoted -> OnlyTheDrivenEngineIsPromoted
FinalizePromotesOnlyTheEngineUnderTest
-> FinalizePromotesOnlyTheDrivenEngine
No configuration selects these by name, so nothing else needed updating.
504 unit tests run, 502 pass (2 skipped, golden data not pulled); the name
validation ctest passes.
JIRA ID: ALMIOPEN-2333 Four review findings, all verified against the code rather than taken at face value. ReferenceOpCoverage: drop the const_cast. GraphWrapper::fromSerializedBlob() already takes a const void*, so the cast stripped const only to hand the pointer straight back to a const parameter. OutputComparison.hpp: include <optional>. std::optional<TensorMismatch> is part of this header's interface and it compiled only through a transitive include from Tensor.hpp. This header is deliberately the pure seam -- no gtest, no config -- so it has to stand alone. main.cpp: reject --enforce-support-claims together with --validate-golden-data. Golden-data validation returns from registerBundleTests() before registerBundles() runs, so no claim test is ever registered; enforcement then seeded its counters, found graphsQueried == 0, and exited 1 through verifiedNothing(), whose message names three causes and none of them is this one. The combination is now refused at startup next to the existing --test-engine check. --validate-golden-data itself is not redundant, despite the review note: it is the only selector for BundleReferenceValidationHarness and it replaced --verification-mode=golden-check, which now throws. Docs: the "Both openGraph() and checkSupportClaims() are virtual" paragraph described the inheritance seam this branch deleted and contradicted the header's own "This class has no virtual members". Replaced with the four injected collaborators and how a deviceless suite wires them. Unit tests: 504 run, 502 pass, 2 skipped (golden data not pulled) -- unchanged from the previous commit. The new rejection exits 1 with its own message; --enforce-support-claims over quick_ConvolutionFwd_Default.1d* still passes 30 tests and exits 0, so the guard does not false-positive.
JIRA ID: ALMIOPEN-2333 Review flagged two duplications in the harness suites. Both were wider than reported, and the second was a live bug. driveHarness(): the SetUp-then-TestBody-under-a-fake-reporter block was written out four times and only TestErrorPaths guarded it. Test::Run() checks IsSkipped() after SetUp() and does not call TestBody() when it is set, so the other three diverged from production semantics the moment a metadata guard or a TOML skip fires in SetUp(). HarnessTestSupport.hpp now owns that block, with the guard. Only the driving is shared. The four suites genuinely disagree on setup -- one passes a support-claim locator, one tags metadata, two build the harness from a verification mode -- and folding those into one signature buys nothing, so each keeps a thin wrapper. TestEnforcementRungs' wrapper became a pure forwarder and is gone; its eight call sites call driveHarness() directly. scratch::makeDir(): temp directories were named from the source line alone, under a shared temp_directory_path(), and reused via remove_all(). Two concurrent runs of this binary draw the same name and the remove_all() deletes the other run's fixture mid-test. This is not theoretical. Six concurrent runs of the unit binary before this change: five passed 502, one failed 2 -- DuplicateSweepCaseIdThrows, UnusedSweepValueWarnsButLoadSucceeds, FlatCustomerBundleDrop. After: eight concurrent runs, 502 each. The review named four sites. There were nine, so all nine now draw from one helper keyed on clock, pid and a counter: TestErrorPaths, TestSupportClaimEnforcement, TestEnforcementRungs, TestVerificationModePaths -- the four reported TestBundleDiscovery, TestBundleVerificationHarness, TestSupportClaims, TestSupportVerdict -- same line-keyed pattern, unreported TestVerificationPaths -- a fixed literal name, no key at all TestTestSettings -- unseeded std::rand(), same sequence per process Nothing removes a path it did not create. ScopedDirectory throws when the name is taken, so a lost race is retried rather than adopted; that is the property the retry loop is built on. TestBundleMetadata already had this helper and its own copy of currentProcessId(), as did TestSupportMatrixCollector -- both now use the shared one. 504 unit tests run, 502 pass, 2 skipped (golden data not pulled), unchanged.
…ence validation
JIRA ID: ALMIOPEN-2333
Review noted that registerReferenceValidationTests() logged how many bundles
fell outside a reference's supported-op set but never which ops were
responsible, and that uncoveredNodeTypes() -- which answers exactly that --
was production-dead, kept alive only by its own unit test. The summary now
names them:
Golden-data validation (GpuRef): 12 bundle(s) registered, 40 without golden
data, 7 outside this reference's supported-op set
(BatchnormInferenceAttributes, ReductionAttributes)
The op set is framed as a commitment in ReferenceOpCoverage.hpp -- leaving an
op out means bundles using it are "simply not validated by that reference,
visibly". A bare tally says a gap exists; the op names say which one to close.
Fixed the contradiction underneath it first. graphNodeTypes() returned an
empty set both for a graph with no nodes and for a buffer that failed
verification, so an unreadable graph was "not covered, but nothing is
uncovered" -- a diagnostic that would report an exclusion with no reason
attached the moment this was wired up. It now returns
std::optional<std::set<NodeAttributes>>, nullopt for unreadable, and
uncoveredNodeTypes() reports K_UNREADABLE_GRAPH for that case.
referenceCoversGraph() is unchanged in behaviour: both empty and unreadable
still mean false.
The string assembly is a separate pure function, formatUncoveredOps(), rather
than inline at the log site. registerReferenceValidationTests() is inline and
reaches sharedReferenceExecutors(), which the unit target deliberately does
not link -- calling it from a test would emit that reference and break the
"these tests never reach an engine or a device" link-time guarantee. Splitting
the formatter keeps the diagnostic testable without spending that property.
507 unit tests run, 505 pass, 2 skipped (golden data not pulled), up from
504/502 by the three new cases; no test dropped. The summary line is
byte-identical to before when nothing is excluded, verified against a real
--validate-golden-data cpu run over 5708 bundles.
JIRA ID: ALMIOPEN-2333 Three review follow-ups. clang-tidy, Linux superbuild: ScratchDirectory.hpp's retry loop left only a comment in its catch, which bugprone-empty-catch rejects under warnings-as-errors, failing four translation units. The helper this was lifted from ended its catch with `continue;` and that statement is what satisfied the check; restoring it. Not reachable locally -- ClangCheck.cmake reports "Skipped creating 'tidy' targets; not available on Windows". RankedEngines now stores hipdnn_frontend::Error whole rather than splitting it into an ErrorCode and a message. GraphSession.hpp already includes Error.hpp, so this costs no new dependency, and Error is a plain copyable struct that default-constructs to OK. isResolved() and enginesAccept() keep taking the bare ErrorCode -- they are pure predicates over it, which is what makes them testable without a frontend object -- so callers pass status.get_code(). buildPlans() ran the same four-line "if bad, return failed with its message" block three times over create_execution_plans/check_support/build_plans. That is now a firstFailure() lambda returning std::optional<EngineOpResult>. A macro was considered and rejected: hiding a `return` behind one costs more in readability than the three blocks did, and the frontend's existing HIPDNN_CHECK_ERROR family returns Error, not EngineOpResult, so none of them applies here anyway. 507 unit tests run, 505 pass, 2 skipped (golden data not pulled), unchanged.
…n binary
JIRA ID: ALMIOPEN-2333
Review found that --validate-golden-data breaks the nightly lane: the workflow
sets HIPDNN_TEST_VERIFICATION_MODE=golden-check, resolveVerificationMode() feeds
it to parseVerificationMode(), which now throws, so every binary in that step
exits 1. The step is schedule-gated, so PR CI stayed green. Reproduced locally:
the env var alone is enough to kill the binary at startup.
The proposed fix was an env fallback for the new flag. Investigating the lane
showed the flag was in the wrong place to begin with. external_integration_test
is applied by add_external_integration_test_target(), which parameterizes a
binary over a provider's plugin and engine. Golden-data validation involves no
engine, so the nightly step ran identical work once per provider -- three lanes,
three chances to disagree about our own data.
So golden-data validation is now its own binary, hipdnn_golden_data_tests,
registered once via add_integration_test_target(). It compiles neither
FrontendGraphEngineRunner.cpp nor HarnessDependencies.cpp, loads no plugin and
creates no handle, which makes "this run cannot reach an engine" a property of
the link rather than of a flag. --validate-golden-data and
TestConfig::goldenDataValidationReference are gone with it, as is the
mutually-exclusive-flags check added earlier in this branch: the combination it
rejected can no longer be spelled. Neither had shipped, so nothing outside this
branch could depend on them.
registerBundleTests() split into a shared detail::discoverAndLoadBundles() plus
two entry points, one per binary. detail::buildVariantPack() moved to
VariantPackBuilder.cpp: both harnesses use it, but it lived in the engine
harness's TU, so the golden-data binary could not link without dragging the
whole engine harness along for one pure function.
Fixed a latent bug found on the way: registration hardcoded requiresDevice=true
for both references, so SetUp() ran SKIP_IF_NO_DEVICES() on the CPU lane even
though useDevice() correctly reports it needs none. CPU golden-data validation
silently skipped on any runner without a GPU. It is now gated on the reference
type.
The nightly step is removed rather than repointed: the new target is an ordinary
ctest target, so the existing "Run tests" step already covers it, matching how
hipdnn_gpu_ref_tests is handled. Note the lane validates nothing today either
way -- the workflow sparse-checks-out .dvc but never runs `dvc pull`, so every
bundle registers as "without golden data". That is pre-existing and worth its
own ticket.
Also in this commit, from the same review round:
- Two ALMIOPEN references reworded functionally (the doc's, new in this PR,
and the pre-existing TODO in IntegrationBundleVerificationHarness.hpp).
- FrontendGraphEngineRunner.hpp's "The real thing" summary replaced with what
it actually implements.
- A per-class responsibility table added to the enforcement doc, plus the
two-binaries split reflected in the scope table, the examples, the symptom
table and the README.
- The retired-mode message no longer names --validate-golden-data, a flag that
no longer exists.
507 unit tests run, 505 pass, 2 skipped, unchanged. hipdnn_golden_data_tests
registers both reference suites and exits 0 with a DVC hint where the engine
binary previously exited 1 on "zero tests ran". Enforcement over
quick_ConvolutionFwd_Default.1d* still passes 30 tests, exit 0.
|
|
||
| The two axes meet in exactly one place — a claim that was accepted and then failed | ||
| in use is reported as such, and never published as working support. See | ||
| [Phase 2](#phase-2--commit-with-the-outcome). |
Contributor
There was a problem hiding this comment.
[Warning] Broken intra-document anchor, and the label names the wrong phase:
See [Phase 2](#phase-2--commit-with-the-outcome).The actual headings are ### Phase 1 — claims, above everything (:159), ### Phase 2 — what the run achieved (:201), ### Phase 3 — commit with the outcome (:218). The link dead-ends, and a reader who scrolls to Phase 2 manually lands on the wrong section.
Should be [Phase 3](#phase-3--commit-with-the-outcome).
Contributor
There was a problem hiding this comment.
This feels so much cleaner. Nothing to be done here, just a kudos.
| // an op speculatively. | ||
| const std::set<NodeAttributes>& cpuSupportedOps() | ||
| { | ||
| static const std::set<NodeAttributes> s_ops = { |
Contributor
There was a problem hiding this comment.
I'm trying to think of how to keep these from going stale. Could it be migrated to a property of the reference executors?
| return _referenceExecutors->get(_referenceType); | ||
| } | ||
|
|
||
| void BundleReferenceValidationHarness::SetUp() |
Contributor
There was a problem hiding this comment.
[Warning] SetUp() drops the TOML skip list and the arch/VRAM metadata guards — a behavioural regression versus the path this harness replaces.
The old golden-check mode ran through IntegrationBundleVerificationHarness::SetUp (base :81-98), which called checkTomlSkip(currentTestName()) and applyMetadataGuards() (checkVramRequirement + checkArchCompatibility). This SetUp() does neither — only SKIP_IF_NO_DEVICES() plus two registration-invariant asserts. Combined with the deliberate no-skip design, a bundle carrying vram_mb: 40000 or an arch restriction is now run on any device and fails hard where it previously skipped.
Second, related effect: the suite registers as suiteName + "_CpuRef" (BundleRegistration.hpp:237) and currentTestName() is test_suite_name() + "." + name(), so any glob in [[tolerance_overrides]] written against the bundle suite name no longer matches. resolveTolerance at :131 then silently falls back to the default tolerance — tighter than intended — with no skip path to absorb the difference.
Both guards are pure TomlGuards.hpp / BundleMetadata.hpp calls already reachable here. Add them, and either keep the un-suffixed name for TOML lookups or document that overrides must be duplicated for _CpuRef/_GpuRef.
| *_bundle->tensors, outputs, wrapper.getTensorMap(), _bundle->outputTensorUids, useDevice()); | ||
| } | ||
|
|
||
| void BundleReferenceValidationHarness::TestBody() |
Contributor
There was a problem hiding this comment.
[Warning] This TestBody() is compiled into the unit-test binary but has no test.
tests/CMakeLists.txt adds BundleReferenceValidationHarness.cpp to hipdnn_integration_tests_unit_tests, yet no TestBundleReferenceValidationHarness.cpp exists. The PR adds tests for GraphSession, OutputComparison, ReferenceOpCoverage, the enforcement rungs and claim enforcement — but not for the one brand-new harness that is now the sole gate on checked-in golden data and, by design, has no skip path.
It is testable as written: _referenceExecutors is an injectable IReferenceExecutors and tests/mocks/MockReferenceExecutors.hpp already exists, so with requiresDevice=false the whole body runs on CPU. Four branches worth covering: inapplicable executor, ReferenceCapabilityError, generic std::exception, and a value mismatch producing exactly one ADD_FAILURE per drifted tensor.
| auto wrapper = _bundle->graphWrapper(); | ||
| const auto& tensorAttrMap = wrapper.getTensorMap(); | ||
|
|
||
| for(const int64_t uid : _bundle->outputTensorUids) |
Contributor
There was a problem hiding this comment.
[Warning] This block hand-reimplements OutputComparison.hpp, which this same PR extracted one commit earlier.
Lines 124-159 duplicate compareOutputs(); lines 136-158 duplicate compareTensor() verbatim (same ComparisonContext fields, same formatComparisonHeader + appendComparisonDiffByDataType calls); lines 142-144 duplicate tensorLabel() (OutputComparison.cpp:14-23). Compare against IntegrationBundleVerificationHarness::compareAgainst() (.cpp:602-638), which does the same job in ~15 lines by consuming the shared helper.
The same file also re-derives allocateSentinelOutputs() at :45-57 and markOutputsModifiedFor() at :108-119 — including the static_cast<void>(uid) at :110, only needed because the loop binds a key it does not use.
This matters beyond tidiness: two copies of the diff-formatting contract will drift, and this copy is the one with no unit test (see the :74 comment). The sharing seam already exists — the variant-pack builder is correctly routed through detail::buildVariantPack — these three just weren't.
Replace 124-159 with bundle::compareOutputs(wrapper, _bundle->outputTensorUids, referenceOutputs, goldenLookup, toleranceFor, contextLine) and ADD_FAILURE() << m.report over the result. Promote allocateSentinelOutputs/markOutputsModifiedFor to free functions next to detail::buildVariantPack; the mark loop then becomes for(auto& [_, tensor] : outputs) with no cast.
|
|
||
| result = graph.execute(handle, variantPack, workspace.get()); | ||
| ASSERT_TRUE(result.is_good()) << result.get_message(); | ||
| if(update.missedQuery) |
Contributor
There was a problem hiding this comment.
[Warning] A from_binary failure is misreported as a harness bug, three times over.
When the graph fails to build, checkSupportClaims() returns a default-constructed SupportObservation (.cpp:66-72), i.e. sidecar == SidecarState::NONE. TestBody() (.hpp:133) then calls recordClaimCoverage() unconditionally. shouldEnforceClaims() is still true (engine injected, sidecar on disk, enforcement on), so coverageFor() sets missedQuery = true (SupportClaimReport.cpp:30) and this ADD_FAILURE() fires.
But the new doc defines that exact message as "A code path short-circuited above the query — a harness bug, not a data problem" (docs/support-claim-enforcement.md:323). Firing it for an ordinary bad-bundle failure sends a maintainer hunting a defect that does not exist — and it is the third message the same failure produces (the ADD_FAILURE at .cpp:70, this one, and the FAIL() from runComparison's outcome).
Second-order damage: update.queried stays false, so graphsWithClaims > graphsQueried and printSupportClaimSummary() (SupportClaimReport.cpp:78-84) attributes the shortfall to --gtest_filter, which is also wrong.
A build failure is not a missed query — the query was impossible, not skipped. Simplest fix:
const auto observation = checkSupportClaims(session);
if(session.buildError.empty())
{
recordClaimCoverage(observation);
}Better: give coverageFor() the graph state so the distinction is testable rather than positional (see the SupportClaimReport.cpp:30 comment). Either way, no test currently drives buildErrorSession() through the real TestBody() with enforcement on — that case is worth adding alongside the fix.
| } | ||
|
|
||
| /// A session whose from_binary failed. | ||
| inline GraphSession buildErrorSession(std::string error) |
Contributor
There was a problem hiding this comment.
[Warning] buildErrorSession() was written for the GraphSession::buildError path and has no callers anywhere under tests/ (verified by grep). Two production branches consequently have zero coverage:
IntegrationBundleVerificationHarness.cpp:66-72— theADD_FAILURE() << "from_binary failed: "plus the earlyreturn {}that suppresses the claim query.IntegrationBundleVerificationHarness.cpp:201-206—VerificationOutcome::failed(NOT_REACHED, FailureOrigin::ENGINE, ...).
Mutation question: delete either branch outright and every test in this suite still passes.
The second one matters most — it decides that a graph which would not load is blamed on the engine, and therefore demotes the claim to CLAIM_FAILED_IN_USE. That classification is exactly what this PR is about. One case in TestEnforcementRungs stubbing openGraph to return buildErrorSession("boom"), asserting the failure, the recorded verdict, and that the claim observer was still consulted, would close it — and would also pin whichever fix you choose for the missedQuery misreporting at IntegrationBundleVerificationHarness.cpp:88.
| } | ||
|
|
||
| /// Collects the reasons the harness recorded as reference errors. | ||
| inline void captureReferenceErrors(::testing::NiceMock<MockVerificationReporter>& reporter, |
Contributor
There was a problem hiding this comment.
[Warning] captureReferenceErrors() is likewise dead — no caller under tests/. IVerificationReporter::recordReferenceError has three call sites (IntegrationBundleVerificationHarness.cpp:300, :339, :361) and no test observes any of them.
TestErrorPaths.RefCrashFails (TestErrorPaths.cpp:181) and TestSupportClaimEnforcement.ReferenceErrorDoesNotDemoteTheClaim (:404) both drive the path but assert only the gtest disposition.
Mutation: remove recordRefError(...) from runExplicitRefMode and nothing goes red — yet the reference-error report, the thing that tells an operator a reference rather than the engine is broken, silently empties. One captureReferenceErrors + ASSERT_EQ(errors.size(), 1u) in each of those two tests closes it.
| /// Both reference executors answer with the golden value: whichever one the | ||
| /// dispatch picks (GPU for explicit `gpu`/`auto`'s first try, CPU for explicit | ||
| /// `cpu`/`auto`'s fallback) matches the engine. | ||
| void useMatchingReference() |
Contributor
There was a problem hiding this comment.
[Warning] useMatchingReference() writes the golden value into both cpuReference and gpuReference, and every explicit-mode test uses it (:258, :286). The docstring at :97-99 is honest about this, but two real behaviours end up unpinned:
- Mutation A: change
IntegrationBundleVerificationHarness.cpp:228soVerificationMode::GPUdispatchesReferenceExecutorType::CPU—DeviceModeRefSucceedsPassesandCpuModeRefSucceedsPassesboth still pass. - Mutation B: swap the GPU and CPU blocks in
runAutoMode(:326-372) —AutoNoGoldenRefSucceedsPassesandAutoNoGoldenRefMissFallsThroughToCpuboth still pass, because the "GPU miss" stub throws a capability error, so a CPU-first order reaches CPU, matches, and returns.
The old hand-rolled RefStub had the same gap, so this is not a regression — but the new DI seam makes it a one-liner to close: EXPECT_CALL(_mocks.referenceExecutors, get(ReferenceExecutorType::GPU)).Times(1) (and get(CPU)).Times(0)) in the explicit-mode tests, plus an ordered expectation or a call-log in the AUTO tests.
Related and also uncovered: runAutoMode distinguishes a GPU capability miss (silent fall-through) from a GPU runtime error (sets gpuRefErrored, calls recordRefError, and emits a materially different message at .cpp:352-358). No test reaches the latter — useCapabilityMissReference and useGpuMissCpuMatchReference only throw ReferenceCapabilityError, and RefBehavior::ERRORS is only ever paired with VerificationMode::CPU. Delete the gpuRefErrored flag and nothing fails.
| case OutcomeStatus::FAILED: | ||
| // An empty message means the failure is already on the record with more | ||
| // detail than this could add — per-tensor diffs from the comparison. | ||
| if(!outcome.message.empty()) |
Contributor
There was a problem hiding this comment.
[Warning] A FAILED outcome with an empty message goes green. This is the exact silent-pass class the PR set out to close.
reportOutcome() only calls FAIL() when the message is non-empty, on the stated assumption that the failure is already on the record. That holds for exactly one producer — comparisonOutcome(false) (.hpp:271), preceded by one ADD_FAILURE() per mismatch (.cpp:634).
It does not hold for engineDidNotRun() on the ERRORED branch (.cpp:255), which forwards run.message verbatim from EngineOpResult::failed(result.get_message()). If the frontend returns a bad status with an empty message — or any IGraphEngineRunner returns EngineOpResult::failed("") — the harness produces OutcomeStatus::FAILED and records nothing. The EXPECT_FALSE guard at .hpp:152 does not fire because it only checks status == PASSED. Meanwhile commitClaims() records CLAIM_FAILED_IN_USE, so the report and the exit code disagree.
No test covers it — every test double uses a non-empty message.
Make the shortcut prove its premise instead of assuming it:
case OutcomeStatus::FAILED:
if(!outcome.message.empty())
{
FAIL() << outcome.message;
}
// The empty-message shortcut is only honest if something else already
// reported. If nothing did, a bare failure beats a green run.
if(!::testing::Test::HasFailure())
{
FAIL() << "failed at " << toString(outcome.depth) << " ("
<< toString(outcome.origin) << ") with no message";
}
return;Alternatively add an explicit bool alreadyReported to VerificationOutcome, set only by comparisonOutcome(), and stop overloading emptiness.
Decision logic: - Report the depth the engine actually reached. EngineOpResult carries plansBuilt, stamped once in execute() past plan compilation, so a failure inside execute() is credited with BUILDABLE instead of collapsing to "the engine never took the graph" -- which denied a buildable bundle the rung it cleared and read identically to a decline. - Add SidecarState::NOT_QUERIED for a graph that never opened. missedQuery now keys on NONE alone, so a corrupt graph buffer no longer also reports "enforcement would have passed without checking" -- enforcement did not pass, the same test is already red. - Name every engine failure. An empty message on a FAILED outcome means "already on the gtest record", which only the comparison can promise; engineDidNotRun() forwarded the runner's message verbatim, so an empty one yielded a green test. - Commit the claim row even when phase 2 throws. The coverage counter is bumped before the bundle runs, so a throw on the way to commitClaims() left the summary a row short with nothing to reconcile against. A harness throw is now a FailureOrigin::HARNESS outcome: red, named, and no evidence against the engine. - Keep the outcome's message in the claim detail, and print detail for unclaimed rows. describeOutcome() says how far the run got; the message says what to do about it. - Require a resolved query before recording drift. chooseVerdict() returned UNCLAIMED_SUPPORT for an unresolved query, contradicting the name and comment of the test pinning that row. - Ask the reference executor whether it wants device pointers rather than hardcoding the CPU/GPU mapping in two harnesses. Structure: - Extract LoadedEngine into its own header. Three collaborator seams pulled <hipdnn_backend.h> in through LoadedEngineTable.hpp, undoing the care taken to keep the deviceless unit binary free of the backend C API. - Move productionPolicy() into its own translation unit so the single CLI-to-behaviour translation is compiled into the unit target. - Rewrite comments that explained the diff rather than the code. Surface: - Drop 'golden-check' from the --verification-mode help; the parser rejects it and golden-data validation is its own binary now. - Fix the dead Phase 2 anchor, the skip-path table cell (the GPU reference suite does skip on a deviceless runner), the detail-section count, and document the no-applicable-claim counter. Coverage: - New cases for LoadedEngineTable::find(), an unpinned run enforcing nothing, BundleReferenceValidationHarness, productionPolicy's field mapping, and each behaviour above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The bundle verification harness reconstructed "what happened" after the fact from GTest globals (
HasFatalFailure(),IsSkipped(),HasFailure()) and two mutable members, which let it publish support-matrix cells that nothing had actually verified and blame the engine for reference-executor defects. This PR makes every stage underTestBody()return aVerificationOutcomevalue instead, so the claim verdict and the test disposition are each decided once from the same facts, then converts the harness from inheritance seams to injected collaborators so the decision logic is unit-testable without an engine or a GPU.JIRA ID : ALMIOPEN-2333
Risk Assessment
Medium risk. This is test-harness and build tooling only — no shipping product code, no public API, ABI, or schema change — but it is the harness that gates every bundle integration test in every provider lane, and it changes what gets published to the support matrix. Coverage is strong locally (502 unit tests, plus a 2562-bundle external integration run), and the residual risk is that validation so far covers a single ASIC, OS, and provider.
ASIC Coverage
Full multi-arch sweep required. The change is ASIC-independent in mechanism, but it rewrites the harness that executes every generic bundle case in every provider lane, so a regression would surface on any target. Two intentional behavior changes are provider- and arch-dependent in when they trigger: an engine failure during execute now carries the frontend's message instead of an empty one, and a provider that declines while compiling plans at the
buildablerung now yields an unverifiable skip instead of escaping as an uncaught exception. Local validation covered Windows / gfx1151 / miopen-provider only, so the remaining families must pass before merge: Linuxgfx94X, gfx950, gfx125Xand Windowsgfx110X, gfx1151.Testing Summary
hipdnnHandle_t, or a real reference executor, making that a link-time guarantee rather than a convention.clang-formatversion.Testing Checklist
pre-commit run --from-ref <base> --to-ref HEAD- Status: Passed (clang-format v18.1.4, black, cmake-lint, bandit)hipdnn_integration_tests_unit_tests- ASICs: gfx1151 - Status: Passed (502 passed, 2 skipped for unpulled golden data, 0 failed; up from 494/492 with no test dropped)hipdnn_integration_tests --test-engine MIOPEN_ENGINE- ASICs: gfx1151 - Status: Passed (2562 passed; 1 failure,quick_ConvolutionWrw_Default.1d_filter3_pad1_stride2_fp32_ncl_pad1_stride2, confirmed pre-existing by reproducing the identical access violation on the unmodified parent commit)ninja hipdnn-integration-tests-check-format/-format- Status: Passed (verified it fails on deliberately malformed input, and that write mode leaves the worktree byte-identical)Technical Changes
VerificationOutcome(status, depth, origin, message) with an orderedVerificationDepthladder shared by the enforcement rungs and the comparison.commitClaims()becomes the only verdict site andreportOutcome()the onlyGTEST_SKIP/FAILsite;_verifiedand_engineRanare deleted.enforcement_levelrather than "the engine ran". Afullbundle that executed and found no oracle stays accepted; abuildablebundle whose plans compiled is now confirmable, which it never was.FailureOrigin, so onlyENGINEandCOMPARISONdemote a claim. A broken reference executor or missing golden data still reddens the run but no longer marks the engine's cell "do not publish".IGraphEngineRunner,IReferenceExecutors,ISupportClaimObserver,IVerificationReporter) plus aHarnessPolicyvalue. The class now has no virtual members and no test subclasses; suites construct the real harness with gmock doubles, matching themocks/MockX.hppconvention the providers already use.enforceAtLevel()direct coverage for the first time. Itsbuildablerung compiled plans against a real graph, so every test previously stubbed the method out and could only assert on routing.dnn-providersclang-format targets, which shelled out to Unixfindand were unrunnable on Windows, and which were created even when the build had just declared clang-format unusable — producing a reachable target with an empty program path. Adopts the Python driverprojects/hipdnnalready uses, and excludes vendoredsrc/third-party/headers the old target would have reformatted.