Need help writing tests for PR #6088 - Testing UI flash

[Neer-rn]

Debugging Doc: Writing Tests for PR #6088

Context

Issue: #5935
PR: #6088

The Problem

I need to write automated tests that:

• FAIL without my fix (the executePendingBindings() calls)
• PASS with my fix

But every test I write passes even without the fix, which means the tests aren't actually catching the bug.

What the Bug Was

When submitting answers in the exploration player, the submitted answer and feedback text would briefly flash the previous card's content before showing the current card's correct content. This happened because:

1. RecyclerView reuses views for performance
2. When a view gets recycled, it still contains old data
3. Data binding updates aren't instant - there's a delay while new content is parsed
4. During that delay, users see the old recycled content

The flash is very brief - only visible when slowing down the video to 0.2x speed.

The Fix

Added executePendingBindings() in two places in StatePlayerRecyclerViewAssembler.kt:

For feedback items:

binding.viewModel = feedbackViewModel
binding.htmlContent = ""  // Clear old content
binding.executePendingBindings()  // Force immediate update
binding.htmlContent = htmlParserFactory.create(...)

For submitted answer items:

binding.viewModel = submittedAnswerViewModel
binding.executePendingBindings()  // Force immediate update
val userAnswer = submittedAnswerViewModel.submittedUserAnswer

This forces the binding to update immediately, clearing the old recycled view content before the HTML parsing delay.

What I've Tried

Attempt 1: Direct Text Content Verification

I wrote a test that goes through multiple questions and verifies the submitted answer text doesn't show content from previous questions.

Test approach:

@Test
fun testStateFragment_multipleQuestions_submittedAnswerDoesNotFlashPreviousQuestionAnswer() {
  // State 2: Submit "1/2"
  typeFractionText("1/2")
  clickSubmitAnswerButton()
  scrollToViewType(SUBMITTED_ANSWER)
  onView(withId(R.id.submitted_answer_text_view))
    .check(matches(withText("1/2")))

  // State 3: Submit "Eagle" - RecyclerView recycles the view
  selectMultipleChoiceOption(optionPosition = 2, expectedOptionText = "Eagle")
  clickSubmitAnswerButton()
  scrollToViewType(SUBMITTED_ANSWER)

  // Try to verify it does NOT show "1/2" from previous question
  onView(withId(R.id.submitted_answer_text_view))
    .check(matches(not(withText("1/2"))))
}

Expected: Test would fail without executePendingBindings() because it would catch the flash of "1/2"

Actual: Test passes even without the fix

Attempt 2: Checking for Empty/Blank State

I thought maybe I could catch the moment when binding.htmlContent = "" is set (in the feedback case).

Problem: Same issue - Espresso waits for idle, so it doesn't see transient states.

Attempt 3: Adding IdlingResource

I considered using a custom IdlingResource to monitor the binding state, but I'm not sure how to implement this properly or if it would even help since the flash happens during the binding update process.

Why This Is Hard to Test

The core issue is that the flash happens in the tiny window between:

1. RecyclerView assigns a recycled view (still has old data)
2. Data binding starts updating (brief delay for HTML parsing)
3. New content appears

This window is maybe 50-100ms. Standard Espresso testing waits for the UI to stabilize, so it only sees state #3 (final state) - never the intermediate flash.

Questions

1. Is it even possible to test this with Espresso? The flash is too fast and Espresso specifically waits for idle state.

2. Should I use a different testing approach?

What I Think Might Work

Looking at the code, maybe I should test that:

• The binding state is synchronized before expensive operations (HTML parsing)
• Or verify execution order: binding update happens before content assignment

But I'm not confident about this approach and I'm not sure how to implement it in the existing test structure.

Request for Help

I'd really appreciate guidance on:

1. Is automated testing feasible for this kind of transient UI flash?
2. If yes, what testing approach should I use?
3. If no, is manual testing with the before/after videos sufficient for this PR?

I've confirmed the fix works manually (visible in the PR videos), so the code change itself is solid. It's just the automated test coverage that I'm struggling with.

---

[BenHenning]

Thanks for this write up @Neer-rn.

One important clarification: we aren't actually using Espresso here, we're using Robolectric directly on JVM to run the test. The Espresso assertion and matching APIs are being used, but the actual logical driver under the surface here is Robolectric (which is really important as that directly ties to the threading model and how synchronization works).

One unknown here is whether the Espresso APIs are actually synchronizing on the main thread, or if the pending transactions are executing due to our own manual synchronization (we have a utility for this called TestCoroutineDispatchers). This is fundamentally a difficult thing to test, and we may find that it isn't reasonably possible, but I think there's a bit more information that you can collect before we come to that decision.

I suggest that you add a lot more logging lines, specifically in all of the following places:

• In each of the places you added the executePendingBindings() call.
• Before and after every single line in the test, every single line in each helper called by the test, and so on until all called lines within the test file have before and after lines (you can ignore unused helpers and other tests, only this one is relevant).
• Run just specifically this test:

bazel test //app/src/sharedTest/java/org/oppia/android/app/player/state:StateFragmentTest --test_filter=testStateFragment_multipleQuestions_submittedAnswerDoesNotFlashPreviousQuestionAnswer

(Important note: this test should be in StateFragmentTest as an FYI--it isn't obvious that's what you're doing here since your test isn't setting up the state fragment correctly so I'm assuming it's been put somewhere else).

Make sure that the lines are very clearly identifying what's being logged so that it's easy to follow along. What we're looking for is specifically when in the test the bindings are being reset.

Now, this won't actually tell us the point where synchronization might happen. That could be done if we had a way to spy on executePendingBindings (and there is a Robolectric way to do this, but it's pretty difficult to tool so we're going to skip it for now). What we'll try instead is adding a synchronous way to observe the previous feedback state to try and see if it's wrong. The easiest way to do this will be to add one detail to each log line you just added: a full print-out of all feedback text at that exact moment (this hopefully catches the problem because Robolectric pauses execution until we tell it to continue*). You can add the following helper and call it in each log statement you added (which may require piping along the Activity):

private fun getAllFeedbackTexts(activity: Activity): List<String> {
  return findAllViewsByIdRecursive(activity.findViewById(android.R.id.content), R.id.feedback_text_view).map { "${it.text}" }
}

private fun findAllViewsByIdRecursive(root: View, id: Int): List<TextView> {
  return when {
    root.id == id -> listOf(root as TextView)
    root is ViewGroup -> (0 until root.childCount).map { root.getChildAt(it) }.flatMap { findAllViewsByIdRecursive(it, id) }
    else -> emptyList()
  }
}

(This may need some imports added, and I didn't test it; I'm hoping that the fragments are correctly linked that this doesn't need any special fragment hopping. A verification is it should start printing empty and eventually become non-empty once the feedback text exists).

* If any Espresso functions synchronize specifically the main thread then we will not actually see the invalid state in the test. That's essentially what we're trying to verify here. We want to see the invalid state at some point from getAllFeedbackTexts since then it's very likely we have a testCoroutineDispatchers.runCurrent() call shortly after that then automatically corrects the text.

[Neer-rn]

Thank you @BenHenning for your suggestions, it helped me a lot! It took me a bit more time than I imagined to do this, but here is what I got.

I did the logging and empirically verified the fix. The bug is observable in Robolectric!

Test Results

WITHOUT the fix (executePendingBindings commented out):

State 2 --> State 3 transition (logs):

[SUBMITTED ANSWER] BEFORE - Current text: '1/2'  // this is from the previous question!                                                                                                     
[SUBMITTED ANSWER] WITHOUT FIX - Current text: '1/2'  // still showing old text                                                                                                             
[FEEDBACK] BEFORE - Current text: 'Correct!'  // also from previous question                                                                                                                
[FEEDBACK] WITHOUT FIX - Current text: 'Correct!'  // old text persists                                                                                                                     
[FEEDBACK] AFTER HTML parsing - Current text: 'Correct!'  // STILL the old text 

Old text persists in recycled views throughout the binding process.

WITH the fix (executePendingBindings active):

State 2 --> State 3 transition (logs):

[SUBMITTED ANSWER] BEFORE - Current text: '1/2'  // old text present                                                                                                                        
[SUBMITTED ANSWER] BEFORE executePendingBindings - Current text: '1/2'  // still old                                                                                                        
[SUBMITTED ANSWER] AFTER executePendingBindings - Current text: ''  // cleared!                                                                                                             
                                                                                                                                                                                              
[FEEDBACK] BEFORE - Current text: 'Correct!'  // old text                                                                                                                                   
[FEEDBACK] AFTER clearing htmlContent - Current text: 'Correct!'  // interesting - still old!                                                                                               
[FEEDBACK] AFTER executePendingBindings - Current text: ''  // now it's cleared 

The fix successfully clears old content before HTML parsing begins.

Key Finding:

Critical observation from logs:

[FEEDBACK] AFTER clearing htmlContent, BEFORE executePendingBindings - Current text: 'Correct!'

Setting binding.htmlContent = "" doesn't immediately update the view - the binding update is deferred. Only executePendingBindings() forces the immediate clearing, which is essential before the HTML parsing delay.

This pattern repeats consistently at every state transition (State 2-->3, State 3-->4).

Automated Testing Challenge

I attempted to write an automated test following patterns from StateFragmentLocalTest.kt:

@Test
fun testStateFragment_submittedAnswerAndFeedback_doNotFlashPreviousQuestionContent_automated() {
  // Capture old feedback text
  var state2FeedbackText = ""
  scenario.onActivity { activity ->
    state2FeedbackText = getAllFeedbackTexts(activity).firstOrNull().orEmpty()
  }

  clickContinueNavigationButton()
  testCoroutineDispatchers.runCurrent()

  // Submit new answer
  selectMultipleChoiceOption(optionPosition = 2, expectedOptionText = "Eagle")
  clickSubmitAnswerButton()
  testCoroutineDispatchers.runCurrent()

  // Try to verify old content doesn't appear
  scenario.onActivity { activity ->
    val currentFeedbackTexts = getAllFeedbackTexts(activity)
    assertThat(currentFeedbackTexts).doesNotContain(state2FeedbackText)
  }
}

Result: Test PASSES both with fix and without the fix.

According to me, by the timetestCoroutineDispatchers.runCurrent() completes and scenario.onActivity checks the views, all pending work (including HTML parsing) has finished. The flash occurs during binding but assertions only see the final state.

This confirms that the transient binding state, while observable in logs, is not easily capturable with standard Robolectric test assertions due to synchronization timing.

Conclusion

From above we can see the bug exits and my solution fixes it but the automated tests can't catch the transient state. I really need help to move ahead, what should be done now based on this findings @BenHenning please help me again.

[BenHenning]

This is a great finding @Neer-rn. In your example test is it the case that you can't actually capture the old state because onActivity always leads to bindings being executed?

I actually used Gemini to double check some of my assumptions here. It makes sense why executePendingBindings is needed here after all. The problem, as I understand it, is that databinding is designed to asynchronously batch updates for performance but RecyclerView expects immediate view changes so that its layout/measure can be done in one step. Databinding is actually forcing RecyclerView to do a lot of extra work here, and that extra work is very likely causing both the flash and performance problems.

Additionally, the reason you can't test this is because ActivityScenario.onActivity is causing a synchronization barrier. The Runnable we pass to onActivity is posted to the activity's Handler which is run in the main thread's Looper. This means in order for the test to proceed ActivityScenario (via Robolectric) needs to advance the main Looper until the Runnable is invoked. The problem is that Choreographer (the thing that does asynchronous UI stuff in Android) will eventually run pending bindings (as I understand it), and basically is an infinite asynchronous loop since it repeatedly posts itself to the main Looper thread. Thus, by scheduling and forwarding the main Looper, we are inadvertently allowing the Choreographer to run and thus execute bindings. It's sort of impossible to test this broken flow in this manner without some really hacky workarounds (i.e. wrapping the binding API itself so that we can introspect it).*

The good news is that we can actually address both problems much more easily than where you're currently working in the codebase, and likely solve other problems app-wide (I very strongly suspect this also includes #5959 but you'll need to check after making the following change): we can solve this by simply updating BindableAdapter to run executePendingBindings after re-binding the view model, that is, call executePendingBindings exactly after this line:

oppia-android/app/src/main/java/org/oppia/android/app/recyclerview/BindableAdapter.kt

Line 213 in 4bda09f

binding.lifecycleOwner = lifecycleOwner

Separately, you can test this in BindableAdapterTest with a contrived UI test (i.e. one that does not use a full activity setup). Here's a rough place to start that I stitched together from several tests suggested by Gemini. Note that I haven't tested it and it will need additional work:

  @Test
  fun testSingleTypeAdapter_withOneViewType_setItem_synchronouslyUpdatesBindings() {
    // NOTE: singleTypeFactory is a new injection for this test.
    val adapter = createSingleViewTypeWithDataBindingBindableAdapter(singleTypeFactory)
    val recyclerView = RecyclerView(context).apply {
      layoutManager = LinearLayoutManager(context)
      this.adapter = adapter
    }
    adapter.data = listOf(STR_VALUE_0)
    recyclerView.measureAndLayout()
    testCoroutineDispatchers.runCurrent()

    // NOTE: This should be the end of test 1 and omitted from test 2.
    val text = recyclerView.lookUpTextWithDataBindingAt(position = 0)
    assertThat(text).isEqualTo(STR_VALUE_0.boundStringValue)

    // Update the RecyclerView without synchronization the main thread.
    adapter.data = listOf(STR_VALUE_1)
    recyclerView.measureAndLayout()

    // Verify that the data was correctly rebound. This will fail if the adapter doesn't synchronously re-execute pending bindings after the update.
    val text = recyclerView.lookUpTextWithDataBindingAt(position = 0)
    assertThat(text).isEqualTo(STR_VALUE_1.boundStringValue)
  }

  private fun RecyclerView.measureAndLayout() {
    val measureSpec = View.MeasureSpec.makeMeasureSpec(/* size= */ 1000, View.MeasureSpec.EXACTLY)
    measure(measureSpec, measureSpec)
    layout(/* l= */ 0, /* t= */ 0, /* r= */ 1000, /* b= */ 1000)
  }

  // NOTE: 'text_view_for_string_no_data_binding' should be renamed to 'text_view_for_string_with_data_binding' in text_view_for_string_with_data_binding.xml. It's wrong today.
  private fun RecyclerView.lookUpTextWithDataBindingAt(position: Int): String? {
    val viewHolder = findVIewHolderForAdapterPosition(position)
    return viewHolder?.itemView?.findViewById<TextView>(R.id.text_view_for_string_with_data_binding)?.text
  }

(This actually is 2 tests in one with suggestions to split it up, and other suggestions for how to make this compile correctly and another general cleanup).

You should see if the fix above works to address the original issue (and also #5959). If it does, work on getting the tests suggested above working and validate that the second one fails correctly when the fix isn't in place and passes when it is. At that point I think you can open a PR to address all of the issues the fix actually solves.

* Caveat: most of this is from my memory of looking at these different parts of code over the past decade, so it may not be precisely correct, but I think it roughly is correct.

[Neer-rn]

@BenHenning so I went ahead and did a proper deep dive into the centralized approach thing. here's what I found:

What I Found in the Code

So basically I traced through the whole code and found out that feedback and submitted answer items use registerViewBinder (lines 1340 & 1448 in StatePlayerRecyclerViewAssembler.kt), NOT registerViewDataBinder.

These two are completely different code paths in BindableAdapter.kt:

Path 1: registerViewBinder (lines 161-165)

override fun bind(data: T) {
  bindView(inflatedView, data)  // just calls the lambda, nothing fancy
}

Path 2: registerViewDataBinder (lines 207-219)

override fun bind(data: T) {
  setViewModel(binding, data)
  binding.lifecycleOwner = lifecycleOwner
  binding.executePendingBindings()  //  the centralized fix sits HERE
}

Testing Results

I tested with ONLY the centralized fix (commented out the specific fixes):
The flash still shows up so the centralized fix alone doesn't fix #5935

I tested with ONLY the specific fixes (removed centralized fix):
The flash is gone so we definitely need the specific fixes for #5935

The Testing Problem

So yeah I'm still stuck on the same issue as before, I can't really write automated tests that fail without the specific fixes because of how Robolectric handles synchronization.

The centralized fix is still useful for other parts of the app that go through registerViewDataBinder like you mentioned #5959, but it just doesn't help with testing #5935 specifically.

I want to properly test this fix, but I'm back to the same challenge that started this discussion. I can't really figure out how to verify my fixes with automated tests. would really appreciate some guidance on how to move forward!

[Neer-rn]

Hey @BenHenning, hope you're doing well! Just wanted to give a friendly ping here, it's been a couple of weeks and I'm still waiting on your thoughts about how to proceed with the testing. The PR is getting marked stale now and I really don't want it to close. Would love any guidance from your side even a small hint on which direction to go would help a lot! Thanks in
advance!

[BenHenning]

@Neer-rn the first bind pathway doesn't use databinding so we wouldn't expect it to be affected by this problem (assuming the original premise that this is an update compatibility issue between databinding & RecyclerView is actually correct). I suspect the execution line I suggested above is still correct, but if it isn't fixing the underlying problem then more investigation is needed. It may be the case that we have some sort of dynamic layout calculation happening that needs to be completely re-triggered because of the notification of updating everything (which is ultimately why data diffing fixes all flashing issues).

I suggest seeing if there are other locations along the data pipeline where executePendingBindings still fixes the issue. Rather than having it exactly where you put it in StateRecyclerViewAssembler see if you can go "deeper" in the binding flow until you eventually get to BindableAdapter and see at which point does it stop working to add that call since this will shed more context. We went from re-running the bindings right at the specific view model bind site all the way down to every view model being bound. We know the former works and the latter (for some reason) doesn't. Seeing precisely where in the binding pipeline this fails will be really helpful to determine where to go next.

[Neer-rn]

@BenHenning hey so I finally figured it out!! so basically earlier when I said your centralized fix wasn't working, I was actually putting executePendingBindings() in the wrong path. I was adding it inside registerViewDataBinder but feedback and submitted answer items use registerViewBinder not registerViewDataBinder, so it was literally never reaching those views. That's why it seemed like it wasn't working.

Then I followed your suggestion of going deeper in the pipeline step by step, and moved executePendingBindings() to registerViewBinder's bind() method in BindableAdapter (after bindView completes), using DataBindingUtil.findBinding()?.executePendingBindings(). Tested on emulator and flash is completely gone!!

So your centralized approach was correct from the beginning, I just had it in the wrong place StatePlayerRecyclerViewAssembler doesn't need any changes now, the fix is cleanly centralized in BindableAdapter itself.

Also added 2 regression tests in StateFragmentTest that verify the correct submitted answer and feedback show up after state transitions. I've pushed the updated PR, please take a look when you get time! And huge thanks for the guidance throughout, really helped a lot.

[BenHenning]

That's an excellent finding, @Neer-rn! I definitely was scratching my head after your earlier message because I was pretty certain it would work, so glad to hear it actually is the correct approach. Well done!

[Neer-rn]

Thanks so much, @BenHenning! Your suggestion were correct rather than where I was looking initially.

The PR is now ready for review: #6088. Would appreciate your eyes on it when you get a chance!