[Test, Tools] (tools/trace_replay): give the retrace harness a SPLIT variant whose output and artifact directories carry the variant too, and assert the transport resolved - a pull library under MOBILEGL_TRANSPORT=inproc replays OpenRA at ssim 1.000, so the picture cannot be the gate

This commit is contained in:
2026-09-11 14:00:14 -04:00
parent eafa47ea46
commit 55ed1c8784
4 changed files with 225 additions and 10 deletions
+78 -8
View File
@@ -322,7 +322,14 @@ function(add_trace_replay_test CASE_NAME BACKEND)
CROP_Y CROP_Y
CROP_WIDTH CROP_WIDTH
CROP_HEIGHT CROP_HEIGHT
COHERENT_AS_FLUSH) COHERENT_AS_FLUSH
# P5. VARIANT is a NAME SUFFIX and TRANSPORT is what the variant runs with; passing
# TRANSPORT without VARIANT is a configure error below, because add_test with a
# duplicate NAME is a hard CMake error and the collision is with the very case this
# entry is a second arm of.
VARIANT
TRANSPORT
IPC_SERVER_PATH)
cmake_parse_arguments(TRACE_CASE "" "${oneValueArgs}" "" ${ARGN}) cmake_parse_arguments(TRACE_CASE "" "${oneValueArgs}" "" ${ARGN})
foreach(required TRACE_ARCHIVE GOLDEN TARGET_CALL WIDTH HEIGHT) foreach(required TRACE_ARCHIVE GOLDEN TARGET_CALL WIDTH HEIGHT)
if(NOT TRACE_CASE_${required}) if(NOT TRACE_CASE_${required})
@@ -347,8 +354,38 @@ function(add_trace_replay_test CASE_NAME BACKEND)
if(NOT TRACE_CASE_CROP_HEIGHT) if(NOT TRACE_CASE_CROP_HEIGHT)
set(TRACE_CASE_CROP_HEIGHT 0) set(TRACE_CASE_CROP_HEIGHT 0)
endif() endif()
# --- P5: the variant suffix, and why the OUTPUT DIRECTORIES have to carry it too ----------
#
# ARCHITECTURE.md:584 asks for the SPLIT suffix because without it the entry collides with
# the same case+backend and add_test with a duplicate NAME is a hard CMake error.
#
# THE DIRECTORIES ARE THE HALF THAT IS EASY TO FORGET AND EXPENSIVE TO GET WRONG. Both arms
# of a case write output/mobilegl.log, and run_trace_case.cmake deletes TRACE_OUTPUT_DIR
# before every run - so two arms sharing one directory means the second arm's run erases the
# first arm's log while `ctest -j` may still be reading it, and, worse, the SPLIT arm's
# refusal census is then a census of whichever arm happened to finish last. That log is the
# ONLY valid census: the console sink is compiled out of these configurations, so a `ctest -V`
# transcript reports a FALSE ZERO for Fatal{ lines. P4a shipped exactly that defect once
# already, in a different place. Same argument for TRACE_ARTIFACT_DIR, which is what CI
# uploads.
set(trace_case_suffix "")
set(trace_case_output_tag "${BACKEND}")
set(trace_case_artifact_tag "actual-images")
if(TRACE_CASE_VARIANT)
set(trace_case_suffix ".${TRACE_CASE_VARIANT}")
set(trace_case_output_tag "${BACKEND}-${TRACE_CASE_VARIANT}")
set(trace_case_artifact_tag "actual-images-${TRACE_CASE_VARIANT}")
elseif(TRACE_CASE_TRANSPORT)
message(FATAL_ERROR
"add_trace_replay_test(${CASE_NAME} ${BACKEND} TRANSPORT ${TRACE_CASE_TRANSPORT}) with no "
"VARIANT: the entry would collide with the monolith arm of the same case and backend, and "
"a duplicate add_test NAME is a hard error")
endif()
set(trace_case_name "MobileGLTraceReplay.${CASE_NAME}.${BACKEND}${trace_case_suffix}")
add_test( add_test(
NAME MobileGLTraceReplay.${CASE_NAME}.${BACKEND} NAME ${trace_case_name}
COMMAND "${CMAKE_COMMAND}" COMMAND "${CMAKE_COMMAND}"
-DTRACE_REPLAY_EXE=$<TARGET_FILE:mobilegl_trace_replay> -DTRACE_REPLAY_EXE=$<TARGET_FILE:mobilegl_trace_replay>
-DMOBILEGL_LIBRARY=${mobilegl_trace_replay_mobilegl_library} -DMOBILEGL_LIBRARY=${mobilegl_trace_replay_mobilegl_library}
@@ -367,17 +404,30 @@ function(add_trace_replay_test CASE_NAME BACKEND)
-DTRACE_CROP_WIDTH=${TRACE_CASE_CROP_WIDTH} -DTRACE_CROP_WIDTH=${TRACE_CASE_CROP_WIDTH}
-DTRACE_CROP_HEIGHT=${TRACE_CASE_CROP_HEIGHT} -DTRACE_CROP_HEIGHT=${TRACE_CASE_CROP_HEIGHT}
-DTRACE_COHERENT_AS_FLUSH=${TRACE_CASE_COHERENT_AS_FLUSH} -DTRACE_COHERENT_AS_FLUSH=${TRACE_CASE_COHERENT_AS_FLUSH}
-DTRACE_OUTPUT_DIR=${CMAKE_CURRENT_BINARY_DIR}/${CASE_NAME}/${BACKEND} -DTRACE_TRANSPORT=${TRACE_CASE_TRANSPORT}
-DTRACE_ARTIFACT_DIR=${CMAKE_CURRENT_BINARY_DIR}/${CASE_NAME}/actual-images -DTRACE_IPC_SERVER_PATH=${TRACE_CASE_IPC_SERVER_PATH}
-DTRACE_OUTPUT_DIR=${CMAKE_CURRENT_BINARY_DIR}/${CASE_NAME}/${trace_case_output_tag}
-DTRACE_ARTIFACT_DIR=${CMAKE_CURRENT_BINARY_DIR}/${CASE_NAME}/${trace_case_artifact_tag}
-P ${MOBILEGL_TRACE_ROOT}/run_trace_case.cmake) -P ${MOBILEGL_TRACE_ROOT}/run_trace_case.cmake)
# The label list gains `retrace-split` on a variant arm, for the same reason
# integration-split exists: a private second label is what lets `ctest -L retrace-split
# --no-tests=error` go RED in a build that never registered the arm, instead of greenly
# running nothing. `retrace` stays so the existing whole-label selections keep describing the
# whole set. A `;` inside a property value has to be escaped.
set(trace_case_labels retrace)
if(TRACE_CASE_VARIANT)
set(trace_case_labels "retrace\;retrace-${TRACE_CASE_VARIANT}")
string(TOLOWER "${trace_case_labels}" trace_case_labels)
endif()
if(BACKEND STREQUAL "DirectGLES") if(BACKEND STREQUAL "DirectGLES")
set_tests_properties(MobileGLTraceReplay.${CASE_NAME}.${BACKEND} PROPERTIES set_tests_properties(${trace_case_name} PROPERTIES
ENVIRONMENT "EGL_PLATFORM=surfaceless;LIBGL_ALWAYS_SOFTWARE=1;MESA_GL_VERSION_OVERRIDE=3.3;MESA_GLSL_VERSION_OVERRIDE=330" ENVIRONMENT "EGL_PLATFORM=surfaceless;LIBGL_ALWAYS_SOFTWARE=1;MESA_GL_VERSION_OVERRIDE=3.3;MESA_GLSL_VERSION_OVERRIDE=330"
LABELS retrace) LABELS "${trace_case_labels}")
else() else()
set_tests_properties(MobileGLTraceReplay.${CASE_NAME}.${BACKEND} PROPERTIES set_tests_properties(${trace_case_name} PROPERTIES
ENVIRONMENT "LIBGL_ALWAYS_SOFTWARE=1;MESA_GL_VERSION_OVERRIDE=3.3;MESA_GLSL_VERSION_OVERRIDE=330" ENVIRONMENT "LIBGL_ALWAYS_SOFTWARE=1;MESA_GL_VERSION_OVERRIDE=3.3;MESA_GLSL_VERSION_OVERRIDE=330"
LABELS retrace) LABELS "${trace_case_labels}")
endif() endif()
endfunction() endfunction()
@@ -386,6 +436,26 @@ function(add_trace_replay_test_for_backends CASE_NAME)
add_trace_replay_test(${CASE_NAME} DirectVulkan ${ARGN}) add_trace_replay_test(${CASE_NAME} DirectVulkan ${ARGN})
endfunction() endfunction()
# --- P5: the SPLIT arm of the retrace matrix -------------------------------------------------
#
# One variant entry per case that trace_cases.json marks `split: true` - today that is OpenRA
# alone, which is the only target the phase gate names and (measurably) the only fixture hydrated
# locally. DirectGLES only: the split shape's first backend is Espryt, and a DirectVulkan arm
# would be measuring a server nobody has written yet.
#
# REGISTERED ONLY UNDER MOBILEGL_BUILD_DISAGGREGATED, because the arm is a statement about a
# library that compiled MG_Remote. A build without the option accepts MOBILEGL_TRANSPORT=inproc
# and silently ignores it (CONTRACT-P5 5), so an unconditional registration would be an entry
# that passes by running monolith - and run_trace_case.cmake's split block would then be the only
# thing standing between that and a green, which is one guard too few for a name that says SPLIT.
function(add_trace_replay_split_test CASE_NAME)
if(NOT MOBILEGL_BUILD_DISAGGREGATED)
return()
endif()
add_trace_replay_test(${CASE_NAME} DirectGLES VARIANT SPLIT TRANSPORT inproc
IPC_SERVER_PATH "${CMAKE_BINARY_DIR}/libMobileGLServer.so" ${ARGN})
endfunction()
set(MOBILEGL_TRACE_CASES_CMAKE "${CMAKE_CURRENT_BINARY_DIR}/trace_cases.cmake") set(MOBILEGL_TRACE_CASES_CMAKE "${CMAKE_CURRENT_BINARY_DIR}/trace_cases.cmake")
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${MOBILEGL_TRACE_ROOT}/trace_cases.json") set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${MOBILEGL_TRACE_ROOT}/trace_cases.json")
execute_process( execute_process(
+96
View File
@@ -28,6 +28,30 @@ if(TRACE_COHERENT_AS_FLUSH)
list(APPEND coherent_as_flush_args --coherent-as-flush) list(APPEND coherent_as_flush_args --coherent-as-flush)
endif() endif()
# --- P5: the transport, threaded as an ENVIRONMENT rather than as a replay CLI flag ----------
#
# MOBILEGL_TRANSPORT reaches the library through the environment, so a `cmake -P` script can set
# it and the child inherits it - exactly how the MOBILEGL_PIPE_VERIFY block at the bottom of this
# file already works, and with no C++ change anywhere. The desktop CLI has no --env option
# (trace_replay_cli.cpp:116-179 is the whole option list) and the Android path already has one
# (trace-replay-ci.sh --env -> trace_replay_core.cpp's setenv block), so an environment read
# covers both directions and a new flag would buy nothing.
#
# TWO WAYS IN, ONE ASSERTION. `-DTRACE_TRANSPORT=` is what the SPLIT ctest variant passes, so
# that entry is self-describing and needs no ritual around it; exporting MOBILEGL_TRANSPORT in
# the calling process is what ~/w7/retrace_gate.py and CI's retrace-split job do over the
# UNCHANGED case names, because the gate parses a FOREIGN reference CTestTestfile.cmake whose
# name regex cannot see a variant suffix. Setting the variable from the -D FIRST means the
# assertions below read one value however it arrived.
if(DEFINED TRACE_TRANSPORT AND NOT "${TRACE_TRANSPORT}" STREQUAL "")
set(ENV{MOBILEGL_TRANSPORT} "${TRACE_TRANSPORT}")
endif()
if(DEFINED TRACE_IPC_SERVER_PATH AND NOT "${TRACE_IPC_SERVER_PATH}" STREQUAL "")
# ARCHITECTURE.md:543. P6 consumes it; P5 carries it so that "unparsed" and
# "parsed and ignored" stop being the same observation.
set(ENV{MOBILEGL_IPC_SERVER_PATH} "${TRACE_IPC_SERVER_PATH}")
endif()
if(EXISTS "${TRACE_OUTPUT_DIR}") if(EXISTS "${TRACE_OUTPUT_DIR}")
file(REMOVE_RECURSE "${TRACE_OUTPUT_DIR}") file(REMOVE_RECURSE "${TRACE_OUTPUT_DIR}")
endif() endif()
@@ -189,3 +213,75 @@ if(DEFINED ENV{MOBILEGL_PIPE_VERIFY} AND NOT "$ENV{MOBILEGL_PIPE_VERIFY}" STREQU
message(STATUS "MGPipe verify: ${pipe_verify_case} armed, zero divergences, zero unmigrated reads") message(STATUS "MGPipe verify: ${pipe_verify_case} armed, zero divergences, zero unmigrated reads")
endif() endif()
endif() endif()
# --- P5: MOBILEGL_TRANSPORT, the split arm's own assertions ----------------------------------
#
# The same problem the verify block above solves, with the same answer and one extra reason to
# need it. A retrace that exported MOBILEGL_TRANSPORT=inproc at a library configured WITHOUT
# -DMOBILEGL_BUILD_DISAGGREGATED=ON is not merely a no-op: the variable's PARSER does not exist
# in that build at all (CONTRACT-P5 5 - putting a complaint in the unconditional part of
# ConfigLoader would move a pull-build symbol and break gate G1), so the value is accepted by the
# environment and silently ignored, every frame still matches its golden, and the case reports a
# clean pass having run monolith end to end.
#
# THE LIBRARY'S OWN LOG IS THE ONLY CHANNEL a `cmake -P` script has for the difference, and it is
# also THE ONLY VALID REFUSAL CENSUS. A `ctest -V` transcript is a FALSE ZERO for Fatal{...}
# lines: the console sink is compiled out of the configurations these lanes run, so the aborts
# reach output/mobilegl.log and nowhere else. That is why TRACE_OUTPUT_DIR and TRACE_ARTIFACT_DIR
# carry the variant - without it both arms write one output/mobilegl.log, the second run wipes
# the first, and the census silently becomes a census of one arm. P4a shipped that defect once
# already; this is the same defect in a new place, pre-empted.
#
# Three demands, all silent when MOBILEGL_TRANSPORT is unset or "monolith":
# * mobilegl.log exists - the replay wrote one, so the library was loaded and logging;
# * it carries ConfigLoader's inproc line, which is emitted ONLY by a build that compiled the
# parser AND resolved the value to InProcess. This is the falsifiable half;
# * it carries no Fatal{ at all. Under split the emit table raises
# Fatal{UnmigratedVerb, "<slot>"} for the 64 slots P5 does not implement, so a clean run of a
# reduced-path target is a run that touched none of them - and any other Fatal{ (ProtocolCorruption,
# UnmigratedPipeInput, AbiMismatch) is a real defect. The count and the distinct names are
# printed either way, because the census is the deliverable even when the run passes.
if(DEFINED ENV{MOBILEGL_TRANSPORT} AND NOT "$ENV{MOBILEGL_TRANSPORT}" STREQUAL "")
set(split_case "${TRACE_CASE_NAME} ${TRACE_BACKEND}")
if("$ENV{MOBILEGL_TRANSPORT}" STREQUAL "monolith")
message(STATUS "MGPipe split: MOBILEGL_TRANSPORT=monolith, no split assertions for ${split_case}")
elseif(NOT EXISTS "${mobilegl_log}")
message(FATAL_ERROR
"MOBILEGL_TRANSPORT=$ENV{MOBILEGL_TRANSPORT} is set for ${split_case} but the run wrote "
"no ${mobilegl_log}, so there is no evidence the transport ever resolved. A split "
"retrace with no library log cannot be counted as a split retrace.")
else()
file(READ "${mobilegl_log}" split_log)
string(FIND "${split_log}" "MOBILEGL_TRANSPORT=inproc" split_armed_at)
if(split_armed_at EQUAL -1)
message(FATAL_ERROR
"MOBILEGL_TRANSPORT=$ENV{MOBILEGL_TRANSPORT} is set for ${split_case} and the library "
"never reported resolving it. ConfigLoader::InitTransport logs one line at INFO when "
"it selects InProcess, and that line exists only in a build configured with "
"-DMOBILEGL_BUILD_DISAGGREGATED=ON - in a build without it the whole parser is "
"compiled out and the variable is accepted and ignored, which is exactly the 'the "
"split lane ran monolith and went green' failure. Check that the SPLIT runtime "
"artifact is the one at ${MOBILEGL_LIBRARY}, and that MOBILEGL_LOG_ACTIVE_LEVEL "
"admits INFO.")
endif()
# The refusal census. Recorded on every split run, pass or fail.
file(STRINGS "${mobilegl_log}" split_fatals REGEX "Fatal\\{")
list(LENGTH split_fatals split_fatal_count)
message(STATUS "MGPipe split: ${split_case} transport=$ENV{MOBILEGL_TRANSPORT}, "
"Fatal{ lines in ${mobilegl_log}: ${split_fatal_count}")
if(split_fatals)
foreach(line IN LISTS split_fatals)
message(STATUS "${line}")
endforeach()
message(FATAL_ERROR
"${split_case}: ${split_fatal_count} MGPipe Fatal(s) under MOBILEGL_TRANSPORT="
"$ENV{MOBILEGL_TRANSPORT}. Fatal{UnmigratedVerb, \"<slot>\"} is one of the 64 emit-table "
"slots P5 leaves unimplemented (R-4) - if the reduced path reached it, either the verb "
"census is wrong or this case is not on the reduced path; Fatal{ProtocolCorruption, ...} "
"is a record that crossed the wire without declaring its bytes (CONTRACT-P5 rule A); "
"Fatal{UnmigratedPipeInput, ...} is a missing row in the field-ownership table. None of "
"them is silenced here: this log is the only place they appear, because the console sink "
"is compiled out of the configurations this lane runs.")
endif()
endif()
endif()
+1
View File
@@ -14,6 +14,7 @@
{ {
"name": "OpenRA", "name": "OpenRA",
"verify": true, "verify": true,
"split": true,
"trace_archive": "openra.tgz", "trace_archive": "openra.tgz",
"trace_file": "openra.trace", "trace_file": "openra.trace",
"golden": "openra.0000031249.png", "golden": "openra.0000031249.png",
+50 -2
View File
@@ -36,6 +36,23 @@ def load_trace_case_manifest(path=TRACE_CASES_JSON):
raise ValueError( raise ValueError(
f"{name} is marked verify but excluded from CI, so the verify matrix would drop it" f"{name} is marked verify but excluded from CI, so the verify matrix would drop it"
) )
# "split" opts a case into the MOBILEGL_TRANSPORT=inproc retrace subset, the same shape
# and the same reason as "verify" above: a typo has to be a loud manifest error in every
# consumer, not a subset that is quietly one case short. The split arm additionally runs
# DirectGLES ONLY - the server the arm exercises is Espryt's - so a case that excluded
# DirectGLES from CI would leave the split matrix with nothing to run.
split = merged.get("split", False)
if not isinstance(split, bool):
raise ValueError(f"split must be true or false for {name}")
if split and not merged.get("ci", True):
raise ValueError(
f"{name} is marked split but excluded from CI, so the split matrix would drop it"
)
if split and "DirectGLES" not in ci_backends(merged):
raise ValueError(
f"{name} is marked split but does not run DirectGLES in CI; the split arm is "
f"DirectGLES-only, so the entry would be registered with no backend"
)
cases.append(merged) cases.append(merged)
return {"defaults": defaults, "cases": cases} return {"defaults": defaults, "cases": cases}
@@ -92,6 +109,16 @@ def verify_trace_cases(cases):
return [case for case in cases if case.get("verify", False)] return [case for case in cases if case.get("verify", False)]
def split_trace_cases(cases):
"""The subset the split (MOBILEGL_TRANSPORT=inproc) CI mode retraces.
P5's phase gate names exactly one: OpenRA, at SSIM >= 0.99. It is also the only fixture that
is hydrated locally, so keeping the subset explicit in the manifest is what stops a later
phase from widening the arm into an LFS fetch by accident.
"""
return [case for case in cases if case.get("split", False)]
def ci_backends(case): def ci_backends(case):
backends = case.get("ci_backends") backends = case.get("ci_backends")
if backends is None: if backends is None:
@@ -122,6 +149,16 @@ def github_verify_matrix(cases):
return github_test_matrix(verify_trace_cases(cases)) return github_test_matrix(verify_trace_cases(cases))
def github_split_matrix(cases):
"""{backend, case} for the split arm. DirectGLES only - see split_trace_cases."""
return {
"include": [
{"backend": "DirectGLES", "case": case["name"]}
for case in split_trace_cases(cases)
]
}
def github_apk_matrix(cases): def github_apk_matrix(cases):
backends = { backends = {
"DirectGLES": {"name": "DirectGLES", "gpu": "software"}, "DirectGLES": {"name": "DirectGLES", "gpu": "software"},
@@ -158,8 +195,8 @@ def emit_cmake(cases, fixture_root):
("CROP_HEIGHT", "crop_height", False), ("CROP_HEIGHT", "crop_height", False),
("COHERENT_AS_FLUSH", "coherent_as_flush", False), ("COHERENT_AS_FLUSH", "coherent_as_flush", False),
] ]
for case in cases: def emit_one(function, case):
lines.append(f"add_trace_replay_test_for_backends({cmake_quote(case['name'])}") lines.append(f"{function}({cmake_quote(case['name'])}")
for cmake_key, json_key, fixture_path in keys: for cmake_key, json_key, fixture_path in keys:
value = case.get(json_key) value = case.get(json_key)
if value is None or value == "": if value is None or value == "":
@@ -169,6 +206,14 @@ def emit_cmake(cases, fixture_root):
lines.append(f" {cmake_key} {cmake_quote(value)}") lines.append(f" {cmake_key} {cmake_quote(value)}")
lines.append(")") lines.append(")")
lines.append("") lines.append("")
for case in cases:
emit_one("add_trace_replay_test_for_backends", case)
# P5's split arm, emitted beside the monolith pair rather than in a block of its own so
# that a case and its variant always carry identical parameters. The CMake side is a
# no-op unless MOBILEGL_BUILD_DISAGGREGATED is ON.
if case.get("split", False):
emit_one("add_trace_replay_split_test", case)
return "\n".join(lines) return "\n".join(lines)
@@ -183,6 +228,7 @@ def parse_args():
"names", "names",
"github-test-matrix", "github-test-matrix",
"github-verify-matrix", "github-verify-matrix",
"github-split-matrix",
"github-apk", "github-apk",
"github-apk-matrix", "github-apk-matrix",
"fixture-files", "fixture-files",
@@ -204,6 +250,8 @@ def main():
print(json.dumps(github_test_matrix(cases), separators=(",", ":"))) print(json.dumps(github_test_matrix(cases), separators=(",", ":")))
elif args.format == "github-verify-matrix": elif args.format == "github-verify-matrix":
print(json.dumps(github_verify_matrix(cases), separators=(",", ":"))) print(json.dumps(github_verify_matrix(cases), separators=(",", ":")))
elif args.format == "github-split-matrix":
print(json.dumps(github_split_matrix(cases), separators=(",", ":")))
elif args.format == "github-apk": elif args.format == "github-apk":
print(json.dumps([github_apk_case(case) for case in cases], separators=(",", ":"))) print(json.dumps([github_apk_case(case) for case in cases], separators=(",", ":")))
elif args.format == "github-apk-matrix": elif args.format == "github-apk-matrix":