Skip to content

fix(tests): register write-watch fault modes once - #13293

Closed
JJenkx wants to merge 1 commit into
boykopovar:mainfrom
JJenkx:codex/write-watch-unique-fault-tests-20261030
Closed

JJenkx wants to merge 1 commit into
boykopovar:mainfrom
JJenkx:codex/write-watch-unique-fault-tests-20261030

Conversation

@JJenkx

@JJenkx JJenkx commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

What

The standalone write-watch CMake project registers write_watch_execute-fault and write_watch_read-fault twice on current main. Configuration stops before either macOS write-watch job can build. Remove the redundant final loop, preserving one registration for each fault mode and all other tests. No C++ test, production code or timeout changes.

Tested

Pinned main 51c31ea17e9f66cb36fe9655ec31c81610bfcfc9, Linux host with CMake 4.4.4/Ninja. Actual unmodified standalone configure fails with both duplicate-name diagnostics; the fixed configure/generate passes and CTest JSON contains 18 unique tests, including each fault mode exactly once. The same before/after result holds in a real Darwin CMake cross-configuration on Linux, with an injected assertion verifying APPLE=1, CMAKE_SYSTEM_NAME=Darwin and CMAKE_HOST_SYSTEM_NAME=Linux. This is registration/configuration proof, not an Apple SDK build or macOS execution.

A Linux compilation attempt of the macOS-only fixtures failed at their unconditional mach/mach.h includes; it is retained as excluded wrong-host evidence. No runtime pass is claimed. Conventions and whitespace pass. The original #12468 macOS CI failures reproduce this main-source configuration error; its published scanner branch already has only one fault loop and is unchanged.

Configure logs, registration manifests, patch and source proofs, SHA-256 8cece16d735c276255db5e3c6617a50957dc86e743d12ccf894c579fb52dacd8 (download verified).

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes (Codex)

@MarcZX10 MarcZX10 left a comment

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.

Reviewed — looks good

The exact-head diff removes only the duplicate execute-fault/read-fault registration loop; the remaining registrations and their 30-second timeout stay intact.

What I ran

Head 75b97c359a77bc7781a44d3e768765abcfd214eb; PR base de2ba4cecb30ffb1c04d7e7dd693b03eace96b1b. A merge preview against current main 42d009f5f37c8f388b01fbfe38d2e704c792c46f was clean (tree c8dffe07254a8804fa5dca9a1a350ef91ee0d13e). Environment: Linux x86_64, Python 3.11.2. I did not configure, build, or run CTest locally.

python3 tools/check_conventions.py --base de2ba4cecb30ffb1c04d7e7dd693b03eace96b1b
git diff --check de2ba4cecb30ffb1c04d7e7dd693b03eace96b1b...75b97c359a77bc7781a44d3e768765abcfd214eb
git merge-tree --write-tree 42d009f5f37c8f388b01fbfe38d2e704c792c46f 75b97c359a77bc7781a44d3e768765abcfd214eb
sha256sum /tmp/anyps5-13293-evidence.tar.gz

Results

Check Result
Conventions Passed.
Diff check Passed; no whitespace errors.
Current-main merge preview Clean.
Evidence artifact Downloaded and SHA-256 matched the PR's stated digest. Its included baseline configure log shows both duplicate-name errors; the fixed configure log succeeds, and its generated CTest manifest contains 18 unique tests with each fault mode once.
Test-quality heuristic Not executable from the isolated PR checkout because the local-only helper script is absent. Manual CMake inspection confirms Recorder.cpp is registered in agc_driver_recorder_tests; this PR changes only the standalone write-watch CMake file.
Hosted CI Intake, eligibility, portability and report passed. Windows, conventions and labels were pending at the latest snapshot; Linux and specialized jobs were skipped.
Unedited local check output
All conventions checks passed.
c8dffe07254a8804fa5dca9a1a350ef91ee0d13e
8cece16d735c276255db5e3c6617a50957dc86e743d12ccf894c579fb52dacd8  /tmp/anyps5-13293-evidence.tar.gz

Negative control

The change adds no assertion or test logic, so I did not run a code-mutation control. The author-provided evidence artifact records the baseline configuration failing on duplicate test names and the changed configuration generating all 18 unique registrations.

Findings

Blocking: None.

Non-blocking: The artifact establishes configuration and test registration, not test execution. The PR accurately states that macOS runtime and Apple SDK compilation were not verified; the current macOS CI jobs were still pending.

Sources

Not verified

No local configure, build, CTest execution, Apple SDK build, or macOS runtime. The cross-configuration and registration evidence is from the verified author-provided artifact; pending hosted checks may add platform evidence.

@adriadam10 adriadam10 self-assigned this Oct 30, 2026
@adriadam10

Copy link
Copy Markdown
Collaborator

Closing as a duplicate of #13220, which was opened earlier and has the identical diff (removes the same redundant foreach(mode execute-fault read-fault) block in core/libs/tests/write_watch/CMakeLists.txt, same blob a912f98cc1). The change itself is correct; it will be reviewed there. Feel free to reopen if #13220 is dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants