Fix #1873: Fix OnboardingFragmentTest test cases on Robolectric #2482

prayutsu · 2021-01-15T13:34:34Z

Explanation

FIxes #1873, fixes #2194, fixes part of #2417
Migrated from ViewPager to ViewPager2
Fixed test cases on robolectric

Testing Video to ensure that new implementation of ViewPager2 does not bring any new regressions -

Screenrecorder-2021-01-15-19-23-34-39.mp4

All test cases passing on Robolectric -

All test cases passing on Espresso -

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

… fix-on_boarding_fragment_test

prayutsu · 2021-01-30T15:47:40Z

@BenHenning @anandwana001 @rt4914 PTAL, the PR is ready for full review.

anandwana001 · 2021-02-01T06:59:40Z

app/src/main/java/org/oppia/android/app/onboarding/OnboardingPagerAdapter.kt

-/** Adapter to control the slide details in onboarding flow. */
+private const val VIEW_TYPE_TITLE_TEXT = 1
+private const val VIEW_TYPE_STORY_ITEM = 2
+
 class OnboardingPagerAdapter(


Are we going to use the Bindale Adapter and remove this Adapter in this PR only or we are keeping this as a separate issue? @rt4914

That seems like a larger & unrelated change--I suggest splitting it off of this task.

Actually, given that so much of the adapter has changed anyway, maybe it is worth trying to migrate to BindableAdapter.

@BenHenning So, should I replace it with a bindable Adapter in this PR only?

Oh, I guess you are suggesting to track it via a new issue.

Given the case that @prayutsu has already worked a lot on this issue, let him decide what he wants to do:
(a.) File an issue, mention it this PR and later on either @prayutsu solves it or the community.
(b.) Use BindableAdapter in this PR itself.

anandwana001

LGTM, Thank @prayutsu

BenHenning · 2021-02-02T03:05:30Z

app/src/sharedTest/java/org/oppia/android/app/onboarding/OnboardingFragmentTest.kt

@@ -529,7 +529,7 @@ class OnboardingFragmentTest {
      }

      override fun perform(uiController: UiController?, view: View?) {
-        (view as ViewPager2).setCurrentItem(position, false)
+        (view as ViewPager2).setCurrentItem(position, /* smoothScroll= */false)


Suggested change

(view as ViewPager2).setCurrentItem(position, /* smoothScroll= */false)

(view as ViewPager2).setCurrentItem(position, /* smoothScroll= */ false)

BenHenning

Overall, I think the new action is reasonable. I agree with @anandwana001's suggestion to move this to BindableAdapter.

Otherwise I have nothing else to add. Feel free to re-add me if you'd like my review, otherwise I'm happy to defer to codeowners for review.

… fix-on_boarding_fragment_test

prayutsu · 2021-02-02T04:45:13Z

Overall, I think the new action is reasonable. I agree with @anandwana001's suggestion to move this to BindableAdapter.

Otherwise I have nothing else to add. Feel free to re-add me if you'd like my review, otherwise I'm happy to defer to codeowners for review.

@BenHenning Thanks for your review, if everything looks good, then can you please approve the changes?

prayutsu · 2021-02-02T04:45:58Z

@rt4914 We Will also need your review, PTAL.

prayutsu · 2021-02-02T04:53:02Z

Filed #2600 for Bindable Adapter Implementation

BenHenning · 2021-02-03T05:17:01Z

@prayutsu I didn't actually review this in detail (and don't need to since I'm not a codeowner). In general, I don't fully review PRs that don't require my review unless the author specifically asks for it.

If you want me to review, I'm happy to, but I might need to prioritize it lower than other reviews that require my approval.

rt4914

@prayutsu Overall this PR has turned out to be really good. Suggested few changes.

app/src/sharedTest/java/org/oppia/android/app/onboarding/OnboardingFragmentTest.kt

rt4914 · 2021-02-03T06:45:57Z

app/src/main/java/org/oppia/android/app/onboarding/OnboardingPagerAdapter.kt

-/** Adapter to control the slide details in onboarding flow. */
+private const val VIEW_TYPE_TITLE_TEXT = 1
+private const val VIEW_TYPE_STORY_ITEM = 2
+
 class OnboardingPagerAdapter(


Given the case that @prayutsu has already worked a lot on this issue, let him decide what he wants to do:
(a.) File an issue, mention it this PR and later on either @prayutsu solves it or the community.
(b.) Use BindableAdapter in this PR itself.

… fix-on_boarding_fragment_test

prayutsu · 2021-02-03T08:27:30Z

@rt4914 I have filed issue #2600 for migration to the bindable adapter, but I am thinking to close that issue and migrate to Bindable Adapter in this PR only. What do you suggest?

rt4914 · 2021-02-03T10:35:28Z

@rt4914 I have filed issue #2600 for migration to the bindable adapter, but I am thinking to close that issue and migrate to Bindable Adapter in this PR only. What do you suggest?

@prayutsu I would suggest you do it in a separate PR.

rt4914

LGTM, thanks.

prayutsu added 30 commits October 1, 2020 15:26

fixed one test

df75643

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

af2fb52

… fix-on_boarding_fragment_test

fix 4 remaining tests

a87d960

try fixing tests bu introducing scrollview

a23aea0

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

b498b4d

… fix-on_boarding_fragment_test

tried adding scrollview

4d46313

fix AndroidManifest.xml

be8405b

fix nit

564d1bd

fix basic set of tests

1e34fa2

fixed 3 failing tests

7e2fbb6

removed extra changes

eabd915

removed extra changes

11a9b61

try fixing tests with testCoroutineDispatchers

543d34c

try advancedUntilIdle()

da4f678

tried migrating to viewpager2

6b7b059

tried implementing new methods

e028aa9

Replace ViewPager with ViewPager2

12540d3

Add ignore annotations

db59608

fix lint

708a488

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

75eac88

… fix-on_boarding_fragment_test

add dependencies to BUILD.bazel

250530b

remove unnecessary ignore annotations

8929252

fix tests by using swipeLeft and swiperight

176dec4

fix lint

8cc353f

fix lint

6555129

fix lint

811e772

fix lint error

5d92e0c

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

f8ce120

… fix-on_boarding_fragment_test

remove redundant lines

06a7a71

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

4a39e40

… fix-on_boarding_fragment_test

prayutsu removed their assignment Jan 30, 2021

anandwana001 reviewed Feb 1, 2021

View reviewed changes

anandwana001 approved these changes Feb 1, 2021

View reviewed changes

BenHenning reviewed Feb 2, 2021

View reviewed changes

anandwana001 removed their assignment Feb 2, 2021

BenHenning reviewed Feb 2, 2021

View reviewed changes

BenHenning assigned FareesHussain and unassigned BenHenning Feb 2, 2021

prayutsu assigned prayutsu and unassigned FareesHussain Feb 2, 2021

prayutsu added 2 commits February 2, 2021 10:07

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

26fb177

… fix-on_boarding_fragment_test

Add a space after argument comment

3023508

prayutsu assigned BenHenning and unassigned prayutsu Feb 2, 2021

prayutsu mentioned this pull request Feb 2, 2021

Migrate to Bindable Adapter for OnBoarding ViewPager2 #2600

Closed

BenHenning assigned prayutsu and unassigned BenHenning Feb 3, 2021

rt4914 suggested changes Feb 3, 2021

View reviewed changes

rt4914 removed their assignment Feb 3, 2021

prayutsu added 2 commits February 3, 2021 13:37

Fix suggested changes.

33a48da

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

1ee0cf5

… fix-on_boarding_fragment_test

rt4914 approved these changes Feb 3, 2021

View reviewed changes

rt4914 merged commit c571a05 into oppia:develop Feb 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1873: Fix OnboardingFragmentTest test cases on Robolectric #2482

Fix #1873: Fix OnboardingFragmentTest test cases on Robolectric #2482

prayutsu commented Jan 15, 2021 •

edited

Loading

prayutsu commented Jan 30, 2021 •

edited

Loading

anandwana001 Feb 1, 2021

BenHenning Feb 2, 2021

BenHenning Feb 2, 2021

prayutsu Feb 2, 2021

prayutsu Feb 2, 2021

rt4914 Feb 3, 2021

anandwana001 left a comment

BenHenning Feb 2, 2021

prayutsu Feb 2, 2021

BenHenning left a comment

prayutsu commented Feb 2, 2021

prayutsu commented Feb 2, 2021

prayutsu commented Feb 2, 2021

BenHenning commented Feb 3, 2021

rt4914 left a comment

rt4914 Feb 3, 2021

prayutsu commented Feb 3, 2021 •

edited

Loading

rt4914 commented Feb 3, 2021

rt4914 left a comment

	(view as ViewPager2).setCurrentItem(position, /* smoothScroll= */false)
	(view as ViewPager2).setCurrentItem(position, /* smoothScroll= */ false)

Fix #1873: Fix OnboardingFragmentTest test cases on Robolectric #2482

Fix #1873: Fix OnboardingFragmentTest test cases on Robolectric #2482

Conversation

prayutsu commented Jan 15, 2021 • edited Loading

Explanation

Checklist

prayutsu commented Jan 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anandwana001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenHenning left a comment

Choose a reason for hiding this comment

prayutsu commented Feb 2, 2021

prayutsu commented Feb 2, 2021

prayutsu commented Feb 2, 2021

BenHenning commented Feb 3, 2021

rt4914 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prayutsu commented Feb 3, 2021 • edited Loading

rt4914 commented Feb 3, 2021

rt4914 left a comment

Choose a reason for hiding this comment

prayutsu commented Jan 15, 2021 •

edited

Loading

prayutsu commented Jan 30, 2021 •

edited

Loading

prayutsu commented Feb 3, 2021 •

edited

Loading