Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SpotlightFragmentTest is very flaky #4775

Closed
BenHenning opened this issue Nov 29, 2022 · 5 comments · Fixed by #4780
Closed

SpotlightFragmentTest is very flaky #4775

BenHenning opened this issue Nov 29, 2022 · 5 comments · Fixed by #4780
Assignees
@JishnuGoyal
Labels
Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

Describe the bug
SpotlightFragmentTest is flaky and is causing incorrect failures both on develop and in individual PR CI runs.

To Reproduce
Steps to reproduce the behavior:

  1. Check out the latest develop (780e367 as of this issue)
  2. Run bazel test //app:src/sharedTest/java/org/oppia/android/app/spotlight/SpotlightFragmentTest --runs_per_test=8
  3. Observe that the test fails a lot (4/8 times for my run)

Expected behavior
The test run should pass for all 8 runs.

Demonstration
image

Environment
Linux Mint 20.3 (Una)

Additional context
None
@BenHenning
Copy link
Member Author

/cc @JishnuGoyal

@BenHenning
Copy link
Member Author

Note that I'm going to run a larger batch of parallel runs (100) to get an idea on a more accurate flakiness percentage.

@BenHenning
Copy link
Member Author

(For Gradle users, you should be able to run the test multiple times locally as well but it may not be as reliable for detecting flakes as using Bazel).

@BenHenning BenHenning added issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged. labels Nov 29, 2022
@BenHenning
Copy link
Member Author

From a local run of 100 tests, 32/100 failed where the following tests were flaky:

  1. testSpotlightQueuing_requestTwoSpotlights_checkFirstSpotlightShown (failed 17 times)
  2. testSpotlightQueuing_requestTwoSpotlights_pressDone_checkSecondSpotlightShown (failed 20 times)

Given that the test suite has 7 tests overall, this indicates a flakiness of ~5.3% among all tests, and per the results, about 32% flakiness per test suite. Individually, test (1) has a 17% chance of failing and test (2) has a 20% chance of failing.

An overall failure rate of 32% is too flaky, so considering this a high priority issue as it will cause a lot of confusion among developers making changes.

@JishnuGoyal JishnuGoyal self-assigned this Dec 4, 2022
@JishnuGoyal
Copy link
Contributor

JishnuGoyal commented Dec 4, 2022 •
edited
Loading

Thanks for providing these details and stats @BenHenning. I'm looking into where this problem is stemming from. Will reach out soon after finding a solution.

@JishnuGoyal JishnuGoyal mentioned this issue Dec 5, 2022
Merged
6 tasks
BenHenning added a commit that referenced this issue Dec 9, 2022
@JishnuGoyal @BenHenning
Fix #4775: SpotlightFragmentTest is very flaky (#4780)
3928c18 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
fixes #4775 

This PR was created to investigate why 2 tests mentioned
[here](#4775 (comment))
were so flaky and fix these issues. From what I found, it seems that the
problem occurs in sequencing while adding spotlight targets:


>checkNotNull(activity.getSpotlightFragment()).requestSpotlight(firstSpotlightTarget)

checkNotNull(activity.getSpotlightFragment()).requestSpotlight(secondSpotlightTarget)


The addition of targets probably lead to a race condition? I haven't
really gotten as deep as to debug it though, but I assumed my
speculation was correct, and an easy fix was to add
`testCoroutineDispatchers.runCurrent()` between these two lines.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

Co-authored-by: Ben Henning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JishnuGoyal JishnuGoyal

Labels
Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #4775: SpotlightFragmentTest is very flaky JishnuGoyal/oppia-android
2 participants
@BenHenning @JishnuGoyal