Skip to content

Fix DoNotMockDataClass lint error: replace ThemeWithEditContext mock with real instance#22835

Merged
jkmassel merged 1 commit intotrunkfrom
jkmassel/fix-theme-mock-lint
May 4, 2026
Merged

Fix DoNotMockDataClass lint error: replace ThemeWithEditContext mock with real instance#22835
jkmassel merged 1 commit intotrunkfrom
jkmassel/fix-theme-mock-lint

Conversation

@jkmassel
Copy link
Copy Markdown
Contributor

@jkmassel jkmassel commented May 4, 2026

Fixes a lint failure on trunk introduced by #22785: EditorSettingsRepositoryTest.mockThemeResponse() mocks uniffi.wp_api.ThemeWithEditContext (a uniffi-generated data class), tripping the DoNotMockDataClass lint rule.

The test only reads theme.isBlockTheme, so a real instance is straightforward.

Summary

  • Replace mock<ThemeWithEditContext>() with a real instance constructed via a buildTheme(stylesheet, isBlockTheme) factory.
  • Factory mirrors the one already in ThemeRepositoryTest.buildTheme.

Why CI didn't catch this on #22785 — root cause

Stale base branch. The chain:

  1. Parallelize CI lint jobs and switch to Debug variant #22637 (merged 2026-04-22) parallelized the lint jobs and switched the PR-time lint variant from Release to Debug in .buildkite/commands/lint.sh.
  2. Add site settings for third-party blocks and theme style gating #22785's branch was based on a trunk commit predating Parallelize CI lint jobs and switch to Debug variant #22637 (git merge-base of the PR head and Parallelize CI lint jobs and switch to Debug variant #22637's merge commit is 61bd1edd6242 — before Parallelize CI lint jobs and switch to Debug variant #22637 landed). The branch was never rebased.
  3. So Add site settings for third-party blocks and theme style gating #22785's PR builds ran the old lint.sh, which executed ./gradlew lintWordpressRelease. Confirmed via build 26166 logs: the only WordPress lint tasks that ran were :WordPress:lintAnalyzeWordpressRelease and :WordPress:lintReportWordpressRelease. No lintAnalyzeWordpressReleaseUnitTest task ran, because AGP's release-variant lint skips the unit test source set by default.
  4. The DoNotMockDataClass violation lives in a unit test file → never analyzed → CI passed.
  5. After squash-merge to trunk, every subsequent PR runs the new lint.shlintWordpressDebug → analyzes unit test sources → catches the inherited violation. PR Skip repeat editor capability fetches in MySiteViewModel #22833 was the first PR branched from post-merge trunk (created 18 minutes after Add site settings for third-party blocks and theme style gating #22785 merged).

(My initial guess was a Gradle build cache reuse issue. Logs ruled that out — the actual run was on Release variant from the stale script.)

Preventing the class of bug — separate ask

The right structural fix is to enable GitHub branch protection's "Require branches to be up to date before merging" on trunk. After enabling, any PR with a stale base must rebase (or merge trunk in) before merging, which forces CI to re-run with the current lint.sh — closing the entire stale-CI-script failure mode.

Cost is real (rebase churn on busy days) but bounded; if a merge queue is in use it's effectively implicit.

I considered also flipping lint { checkTestSources = true } in build.gradle so that trunk-side Release-variant lint covers test sources too. Verified locally: it surfaces 27 pre-existing test-source violations (mostly deliberate test patterns: passing null to non-nullable LiveData, hardcoded resource ints in fixtures, etc.) that would need baseline reconciliation across all four lint variants. That's a separate, larger PR — branch protection alone covers the actual reported bug pattern.

Drive-by note (not in this PR)

Lint output reports 13 unmatched baseline entries (DoNotMockDataClass ×8, DoNotMockSealedClass ×5) — a baseline cleanup is overdue but out of scope here.

Test plan

  • ./gradlew :WordPress:lintWordpressDebug — passes
  • ./gradlew :WordPress:testWordpressDebugUnitTest --tests "org.wordpress.android.repositories.EditorSettingsRepositoryTest" — passes
  • CI lint-wordpress and lint-jetpack green
  • After this lands, rebase Skip repeat editor capability fetches in MySiteViewModel #22833 and confirm its lint passes
  • Follow-up: enable "Require branches to be up to date" on trunk

…sRepositoryTest

The DoNotMockDataClass lint check flags mocking uniffi-generated data classes.
Construct a real instance via a buildTheme() factory matching the pattern
already used in ThemeRepositoryTest.
@dangermattic
Copy link
Copy Markdown
Collaborator

dangermattic commented May 4, 2026

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr22835-843d36a
Build Number1488
Application IDorg.wordpress.android.prealpha
Commit843d36a
Installation URL4r7qhuu0c1vd8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr22835-843d36a
Build Number1488
Application IDcom.jetpack.android.prealpha
Commit843d36a
Installation URL0vliq8ille668
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jkmassel jkmassel merged commit 57acb95 into trunk May 4, 2026
27 of 28 checks passed
@jkmassel jkmassel deleted the jkmassel/fix-theme-mock-lint branch May 4, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants