Skip to content

fix(core): require explicit mock adapter construction - #266

Merged
kolkov merged 3 commits into
gogpu:mainfrom
besmpl:besmpl/no-implicit-mock
Jul 22, 2026
Merged

fix(core): require explicit mock adapter construction#266
kolkov merged 3 commits into
gogpu:mainfrom
besmpl:besmpl/no-implicit-mock

Conversation

@besmpl

@besmpl besmpl commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Remove only the implicit fabricated mock fallback:

  • core.NewInstance now remains empty when no registered provider yields an adapter
  • RequestAdapter reports that failure instead of returning a fabricated mock adapter
  • deterministic tests opt in with NewInstanceWithMock
  • existing explicit software/noop registration remains available
  • hal/allbackends keeps its current desktop software-backend registration behavior

The public constructors and explicit fallback providers remain available.

Why

A configured provider set that yields no adapter could previously look like successful GPU initialization because core fabricated a test adapter. That masks loader, registration, and platform-support failures.

Failing closed makes provider availability observable while preserving explicit software and mock use.

Validation

Go 1.25.12:

  • go test ./core ./hal/allbackends .

Go 1.26.5:

  • go test -race ./core .
  • go vet ./core ./hal/allbackends .

Focused tests cover empty-provider failure and explicit mock construction.

This correction is independent of the Android backend preview.

Android integration relationship

This Android-independent change is one of five prerequisites replayed in #268. Its canonical candidate head is e65bec3c74c56cd07265c055ce6a7ec7fbedc680; the corresponding replay in #268 ends at a093f8e969f99f55dfcffdb4b8bcba61cc13299a. The #268 description records the current exact integration head and full replay map; it drops this replay after this PR merges. Rust-v29 typed surface parity remains separate in #273. Android runtime and release claims remain in #268.

@besmpl

besmpl commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Stacked consumer preview: #268 at exact head b11d2671e80081a191705c41c4e77e8b61472017 incorporates this PR's head e97e4901ee76d9e5f587569c073b38114221c4e6 (matching stable patch IDs).

The preview remains explicitly draft/non-mergeable and will be rebased to Android-only commits after this prerequisite lands.

@besmpl
besmpl force-pushed the besmpl/no-implicit-mock branch from e97e490 to 6baaad4 Compare July 19, 2026 07:14
@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@besmpl
besmpl force-pushed the besmpl/no-implicit-mock branch from 6baaad4 to e65bec3 Compare July 19, 2026 08:36

@kolkov kolkov 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.

Eliminating the implicit mock fabrication is the right call — matches Rust wgpu (no mock/noop fallback in Instance::new) and makes registration failures observable rather than silently masked.

Code is correct, tests are thorough, CI clean on all platforms.

Should fix before merge

hal/allbackends/doc.go is stale after the second commit.

The first commit deleted register.go and updated doc.go to say "The software and no-op providers are not registered by this package." The second commit restored register.go (re-adding _ "github.com/gogpu/wgpu/hal/software") but did not update doc.go. Currently the doc claims software is not registered, but register.go imports it.

Suggested: restore the software backend to the registered list in doc.go and keep only noop as "not registered by this package."

Minor (follow-up OK)

After this PR, useMock is true only via NewInstanceWithMock, which never populates deferredGLES. So the mock-removal block in enumerateDeferredGLES (lines 407-409), hasRealAdaptersLocked, and removeMockAdaptersLocked become unreachable dead code. Not blocking — worth a cleanup pass later.

The new TestNewInstanceDoesNotFabricateAdapterWithoutProvider and TestNewInstanceUsesRegisteredProviderWithoutEnablingMock tests cover the key invariants well. LGTM after the doc fix.

@besmpl

besmpl commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@kolkov Addressed both the requested documentation correction and the optional cleanup in 3e6c836:

  • hal/allbackends now accurately documents software as registered and noop as explicit-only;
  • removed the now-unreachable deferred-GLES mock removal path and its dead helpers.

The branch is mergeable and all 10 repository Actions checks are green. Could you please take another look when convenient?

@kolkov kolkov 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.

Both items addressed in 3e6c836:

  1. doc.go — software correctly listed as registered, noop as explicit-only
  2. Dead code removed — hasRealAdaptersLocked, removeMockAdaptersLocked, deferred-GLES mock block (-41 LOC)

CI green.

@kolkov
kolkov merged commit 8238c4f into gogpu:main Jul 22, 2026
11 checks passed
kolkov added a commit that referenced this pull request Jul 22, 2026
* docs: update CONTRIBUTING.md — Smart Coding framework, AI-assisted policy, current project structure

* docs: CHANGELOG for merged prerequisite PRs (#264, #265, #266, #267, #269)
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.

2 participants