326: Add RTL screenshot test sample

[DanielJette]

What does this change accomplish?

Resolves #326

How have you achieved it?

An RTL screenshot test plus its recorded baseline in Samples/Flix, so the RTL path is exercised on every run.

Notice

Warning

This change must keep main in a shippable state; it may be shipped without further notice.

[AndroidTestifyBot]

Changes requested

Verified locally against the attached emulator (device key 37-1080x2220@440dp-en_US, matching the baselines) rather than on CI. The RTL coverage itself is good and the production refactor is clean — but the two new tests are order-dependent and will fail on CI as currently configured.

---

1. Blocking: the new tests fail under the sharding CI uses

The tests pass in isolation but not as part of the suite:

Run	Result
-PtestClass=…HomeScreenTest (isolated)	OK (2 tests)
:FlixSample:screenshotTest (full suite)	2 of 22 failed — both new tests
Same again after screenshotClear	2 of 22 failed — reproducible

So the committed baselines were recorded from an isolated run. Re-recording from a full-suite run does fix the full-suite case — screenshotRecord rewrote exactly 2 baselines and the suite then went 22/22 — but that isn't enough, because .github/workflows/flix_sample.yml shards Flix into two (shard_count: 2, shard_index: [0, 1]). Replicating that against the freshly recorded baselines:

./gradlew :FlixSample:screenshotTest -PshardCount=2 -PshardIndex=0
  Running test shard 0 of 2...
  1) homeScreenBottomSheet_RTL(…HomeScreenTest)
  Tests run: 12,  Failures: 1

Re-recording alone can't fix this: whichever shard HomeScreenTest lands in has a different set of preceding tests than any local run.

Root cause: the Coil-loaded poster, not the RTL work

I localized the diff between the committed baseline and the full-suite capture:

Region	Differing pixels
Poster (AsynchronousImage / Coil)	85,894 of 116,200 — 74%
Everything else (title, year, overview)	0 of 280,000 — exact

Overall RMSE is only ~1%, so it's a slightly different decode/scale of the same bitmap, not a layout change — consistent with Coil serving from a warm memory cache when earlier tests have already loaded posters. Your text and layout render perfectly deterministically.

Verified fix

Setting posterUrl = null in dummyData() removes the nondeterminism. With baselines recorded in isolation, all three configurations then pass:

:FlixSample:screenshotTest                        → OK (22 tests)
:FlixSample:screenshotTest -PshardCount=2 -PshardIndex=0 → OK (12 tests)
:FlixSample:screenshotTest -PshardCount=2 -PshardIndex=1 → OK (10 tests)

MovieThumbnail wraps the image in a fixed 100.dp × 150.dp Box, so dropping the URL doesn't shift the layout — which is what the test is actually about. An exclusion rect over the thumbnail, or a lower exactness, would also work; the null poster is just the simplest and keeps the test focused on RTL.

---

2. The explicit LocalLayoutDirection override defeats the test's purpose

homeScreenBottomSheet_RTL sets the locale and forces the direction:

overrideResourceConfiguration<ComposableTestActivity>(locale = Locale.forLanguageTag("fa"))
…
CompositionLocalProvider(LocalLayoutDirection provides LayoutDirection.Rtl) { … }

Two experiments against the committed RTL baseline:

• Removed the CompositionLocalProvider entirely → still passes, pixel-identical. The locale override alone already produces RTL.
• Forced LayoutDirection.Ltr instead → ScreenshotIsDifferentException. So the layout genuinely is direction-sensitive and the baseline really does capture mirrored output.

Together those show the provider is a no-op here — and that it's actively harmful to the test's value. Because it forces RTL unconditionally, the test would keep passing even if Testify's locale → layout-direction propagation regressed entirely, which is precisely the mechanism #326 asked this test to guard ("that an RTL locale actually mirrors the layout through Testify's ResourceWrapper/WrappedLocale override").

Please drop the CompositionLocalProvider wrapper (and the now-unused LocalLayoutDirection/LayoutDirection/CompositionLocalProvider imports). The test becomes both simpler and a real regression guard.

Incidentally this is a nice result for the docs: it empirically confirms the claim in #323 that Android derives layout direction from the locale — it holds for Compose through Testify's override, on API 37.

---

What's verified good

• The fa baseline folder validates the documented naming. 37-1080x2220@440dp-fa — bare fa, no country segment, exactly as #323 describes and distinct in shape from the existing fr_FR/ja_JP folders. That's claim 2 of #326 confirmed by a real recording.
• The HomeScreen.kt refactor is faithful. I compared the extracted HomeScreenBottomSheetBody against the original inline block — identical modifiers, identical structure, identical typography and maxLines. Hoisting sheetState with a default preserves behaviour, and adding @OptIn(ExperimentalMaterial3Api::class) to LoadedHomeScreen is the correct consequence of exposing SheetState in the signature. The other 20 Flix baselines were untouched by it.
• Conventions match. No @RunWith matches the other Flix tests; androidx.test:core-ktx:1.4.0 matches Samples/Legacy/build.gradle; the license header and the no-trailing-newline strings.xml style match the existing files. I checked each of these before flagging, and none are findings.
• Farsi strings scoped to src/debug/res is a tidy way to keep them out of the release app.

---

Happy to re-run any of this once updated. Everything above was local — no CI spend.

Reviewed by Claude on behalf of @DanielJette.

[AndroidTestifyBot]

Investigation: why the poster is unstable, and a fix that keeps it

Dropping the poster asset isn't acceptable — agreed, and my earlier suggestion to null it was the wrong call. I dug into the actual mechanism. Superseding that recommendation: there's a one-line fix that keeps the poster.

All of the below ran locally against 748b2609 on a 37-1080x2220@440dp-en_US emulator. No CI.

There are two different failures, not one

That's why the tolerance didn't help.

Failure A — poster present but resampled differently. MoviePosterScreenshotTest loads the same asset (the-man-who-knew-too-much-1934.jpg), and ...ui.common.composables sorts before ...ui.homescreen, so it runs first and seeds Coil's memory cache. MoviePoster renders at screenHeight * screenHeightFrac; MovieThumbnail is a fixed 100.dp × 150.dp. Coil (Precision.AUTOMATIC) reuses the larger cached bitmap and downscales it rather than decoding fresh at thumbnail size. Measured against the committed baseline:

Poster region	85,894 / 116,200 px differ (74%)
Text region	0 / 280,000 px differ
Poster channel delta	mean 1.9, max 80, std 3.1

Mean ~2/255 is invisible; the max of 80/255 is not. That's resampling, exactly as you suspected it shouldn't be — and it only happens because of the size mismatch, not the cache hit as such.

Failure B — poster entirely absent. This is the one that defeats exactness. In shard 0 the capture has no poster at all: 111,495 px differ, and the text is pixel-identical. Blank white against a dark poster is Delta E ≈ 100, so no tolerance absorbs it. exactness = 0.8f was being asked to paper over a missing image, which is why it didn't help.

Failure B is the real bug. Failure A is a symptom of the cache masking it: a cache hit draws near-synchronously, so the poster makes it into the frame (just resampled). With a cold or disabled cache the decode is genuinely async and sometimes loses the race.

Why nothing waits for the load

This is the ScreenshotScenarioRule idling gap from #317. ComposableScreenshotScenarioRule calls composeTestRule.waitForIdle(), which waits for composition — not for Coil's background decode. And unlike ScreenshotRule, the scenario path never calls Espresso.onIdle(), so Coil's IdlingThreadPoolExecutor registration has nothing driving it.

Worth flagging separately: setSynchronousImageLoader() in FlixLibrary/src/androidTest does not work on the scenario path for this reason. It works for CastDetailScreenshotTest because that uses ComposableScreenshotRule, which goes through ScreenshotRule → EspressoHelper → Espresso.onIdle(). (It's also not visible from Samples/Flix androidTest — FlixSample only has implementation project(":FlixLibrary").)

What I tried

Baselines re-recorded in isolation for each variant, then full suite + both CI shards:

Variant	Full	Shard 0	Shard 1
As submitted	❌ 2 fail	❌ 1 fail	✅
Re-record from a full-suite run	✅	❌ 1 fail	—
memoryCachePolicy(DISABLED)	✅	❌ 1 fail	✅
IdlingThreadPoolExecutor + Espresso.onIdle() in a beforeScreenshot observer	✅	❌ 1 fail	✅
dispatcher(Dispatchers.Unconfined)	✅ 22/22	✅ 12/12	✅ 10/10

Answering your questions directly:

• Purge the cache? Works for Failure A, makes Failure B worse — every load becomes a fresh async decode, so it loses the race more often. Not sufficient alone.
• Pre-warm? Fragile. Coil's memory-cache key includes the resolved request size, so a pre-warm would have to reproduce the thumbnail's exact pixel dimensions (100×150dp at 440dpi) to produce a hit rather than a second decode.
• Make sharded and unsharded behave the same? Yes — remove the asynchrony rather than trying to control cache state.

The fix

@Before
fun before() {
    Coil.setImageLoader(
        ImageLoader.Builder(InstrumentationRegistry.getInstrumentation().targetContext)
            .dispatcher(Dispatchers.Unconfined)
            .build()
    )
}

Dispatchers.Unconfined runs the decode eagerly on the calling thread, so the bitmap is ready before the frame is captured — no race, and no dependence on what ran earlier. Memory cache left at its default, so the poster still renders normally.

Verified stable: full suite and shard 0 each run twice more, all green. And the recorded baseline genuinely contains the poster — correctly mirrored to the right, Farsi text right-aligned — so this isn't passing by consistently rendering nothing.

Note it's idling-concurrent:3.7.0 that's already on Samples/Flix (line 112), so no new dependency is needed for the Dispatchers.Unconfined variant either.

Also worth considering

Given two Flix tests now load the same asset at very different sizes, pinning Precision.EXACT on the test ImageLoader would additionally guarantee the decode size, independent of ordering. Not needed for the tests to pass — Unconfined alone is sufficient — but it would make Failure A structurally impossible.

I saw your local WIP moving to exactness = 0.8f and resetting the locale on the non-RTL test. The locale reset looks right; the tolerance won't help on its own, for the Failure B reason above. Happy to re-run the matrix once you push.

Reviewed by Claude on behalf of @DanielJette.

[AndroidTestifyBot]

Approved

Both blocking items are fixed, and I re-verified locally against the committed baselines on c3f15ae6 — nothing re-recorded, git status clean after every run.

Flakiness is gone

Configuration	Result
-PtestClass=…HomeScreenTest (isolated)	OK (2 tests)
:FlixSample:screenshotTest (full suite)	OK (22 tests)
-PshardCount=2 -PshardIndex=0 (as CI runs it)	OK (12 tests)
-PshardCount=2 -PshardIndex=1	OK (10 tests)

Repeated the full suite and shard 0 three times each — green every time. Shard 0 was the configuration that failed on every previous variant, including after re-recording from a full-suite run, so that's the meaningful one.

The Dispatchers.Unconfined ImageLoader in @Before is the right fix: the decode runs eagerly on the calling thread, so the bitmap is ready before capture and the result no longer depends on what ran earlier or on Coil's cache state.

The test is now a real regression guard

With the CompositionLocalProvider gone, the mirroring has to come from overrideResourceConfiguration(locale = Locale.forLanguageTag("fa")) — which is exactly what #326 asked this test to cover. Confirmed from the committed baselines:

Baseline	Left half	Right half
…@440dp-en_US (LTR)	mean 179.9 / std 51.1 ← poster	mean 248.3 / std 26.7
…@440dp-fa (RTL)	mean 251.6 / std 18.8	mean 180.3 / std 51.3 ← poster

Near-identical poster statistics on opposite sides. So the poster is genuinely present in both (not passing by rendering nothing, which was the risk with the earlier cache-disabled variants), and the layout really does mirror. If Testify's locale → layout-direction propagation ever regressed, this test would now catch it.

Adding overrideResourceConfiguration(locale = Locale.getDefault()) to the non-RTL test is a good defensive touch — JUnit doesn't guarantee method order, so without it homeScreenBottomSheet could inherit fa if the RTL test ran first.

Also verified

• The HomeScreen.kt extract-composable refactor is faithful — identical modifiers, structure, typography and maxLines — and the other 20 Flix baselines pass untouched, so no visual regression from hoisting sheetState or adding the @OptIn.
• The main/res/values/strings.xml change is a trailing newline only, no string content change.
• Imports are clean: no orphans left from the removed CompositionLocalProvider, and everything newly imported is used.
• The fa baseline folder (bare fa, no country segment) is a real-world confirmation of the folder naming documented in #323.

One observation, not a request: ComposableScreenshotScenarioRule(exactness = 0.9f) passes the value that's already the default, so it's a no-op — though stating the tolerance explicitly in a sample is arguably self-documenting, so I'd leave it.

Worth a follow-up issue

Two things this surfaced that outlive this PR:

1. setSynchronousImageLoader() silently doesn't work on the scenario path. It relies on an Espresso idling resource, and ScreenshotScenarioRule never calls Espresso.onIdle(). It works for CastDetailScreenshotTest only because that uses ComposableScreenshotRule, which routes through EspressoHelper. Anyone copying that helper into a scenario-based test will hit exactly the race this PR just fixed.
2. Two Flix tests load the same poster asset at very different sizes (MoviePoster at screenHeight * frac vs MovieThumbnail at 100×150dp), which is what made Coil's cache reuse produce different resampling. Precision.EXACT on a shared test ImageLoader would make that structurally impossible.

Happy to file either or both if useful.

All verification was local — no CI spend.

Reviewed by Claude on behalf of @DanielJette.