Fix #3099: Test suite for DrawableBindingAdapters #4077

rishidyno · 2022-01-04T15:14:11Z

Fix #3099

Explanation

Essential Checklist

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: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

… test-suit-for-drawablebindingadapters

github-actions · 2022-01-04T15:15:25Z

Thanks for submitting this pull request! Some main reviewers
have taken time off for the next few weeks, so it may take a
little while before we can look at this PR. We appreciate your
patience while some of our team members recharge. We'll be fully
returning on 4 January 2021.

rishidyno · 2022-01-04T15:51:35Z

@rt4914 @BenHenning last three BindingAdapter functions in are not being uses in DrawableBindingAdapters. Should I remove them?

BenHenning · 2022-01-05T04:11:34Z

@rt4914 @BenHenning last three BindingAdapter functions in are not being uses in DrawableBindingAdapters. Should I remove them?

@rishidyno that should be fine.

… test-suit-for-drawablebindingadapters

rishidyno · 2022-01-13T19:54:12Z

@rt4914 @BenHenning all test passed PTAL.

BenHenning

Thanks @rishidyno. Took a first pass review and left some comments.

For your main PR comment:

Please add an explanation that includes a justification for the test file exemption change (you checked that item the checklist but didn't actually fill in the explanation)
Please add a video showing the new test suite running on an emulator or real device via Espresso, and a screenshot showing that the test passes

app/src/sharedTest/java/org/oppia/android/app/databinding/DrawableBindingAdaptersTest.kt

app/src/main/java/org/oppia/android/app/testing/DrawableBindingAdaptersTestActivity.kt

app/src/main/java/org/oppia/android/app/databinding/DrawableBindingAdapters.java

app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt

app/src/main/AndroidManifest.xml

app/src/sharedTest/java/org/oppia/android/app/databinding/DrawableBindingAdaptersTest.kt

rt4914

@rishidyno PTAL thanks.

rt4914 · 2022-01-17T19:25:49Z

app/src/main/res/layout/test_drawable_binding_adapter_activity.xml

+    android:layout_width="match_parent"
+    android:layout_height="320dp" />
+</LinearLayout>
+


Remove this blank line

rt4914 · 2022-01-17T19:26:43Z

app/src/sharedTest/java/org/oppia/android/app/databinding/DrawableBindingAdaptersTest.kt

+    Intents.release()
+  }
+
+  // from a color int


This comment does not make sense. Can you please make it more clear ?

This means that we are declaring an integer color. Makes sense @rt4914 ? I think we don't need this, so removed it.

app/src/sharedTest/java/org/oppia/android/app/databinding/DrawableBindingAdaptersTest.kt

… test-suit-for-drawablebindingadapters

rishidyno · 2022-01-18T09:49:13Z

Thanks @rishidyno. Took a first pass review and left some comments.

For your main PR comment:

Please add an explanation that includes a justification for the test file exemption change (you checked that item the checklist but didn't actually fill in the explanation)

I added DrawableBindingAdaptersTestActivity to test file exemption because we don't need to test them. Right?

Please add a video showing the new test suite running on an emulator or real device via Espresso, and a screenshot showing that the test passes

oppia-android.DrawableBindingAdaptersTest.kt.oppia-android.app.2022-01-18.14-48-08.mp4

rishidyno

@BenHenning PTAL.

app/src/main/java/org/oppia/android/app/databinding/DrawableBindingAdapters.java

app/src/sharedTest/java/org/oppia/android/app/databinding/ImageViewBindingAdaptersTest.kt

rishidyno · 2022-01-26T14:16:18Z

app/src/main/java/org/oppia/android/app/testing/DrawableBindingAdaptersTestActivity.kt

+import org.oppia.android.app.activity.ActivityComponentImpl
+import org.oppia.android.app.activity.InjectableAppCompatActivity
+
+/** Test activity for DrawableBindingAdapters. */


@anandwana001 Actually, I checked for other data binding test activities but none of them are doing this, so I just left as it is. It can be confirmed if you look into these files. I have linked these 2, but there are more.
https://github.com/oppia/oppia-android/blob/3a0afb4eb8222d4ff3acd2e2e660b85911d35229/app/src/main/java/org/oppia/android/app/testing/StateAssemblerMarginBindingAdaptersTestActivity.kt#L16-L15

https://github.com/oppia/oppia-android/blob/3a0afb4eb8222d4ff3acd2e2e660b85911d35229/app/src/main/java/org/oppia/android/app/testing/ViewBindingAdaptersTestActivity.kt#L16-L15

@BenHenning Please confirm what to do.

… test-suit-for-drawablebindingadapters

BenHenning

Thanks @rishidyno. This LGTM for codeowners.

BenHenning · 2022-01-26T19:37:59Z

app/src/main/java/org/oppia/android/app/testing/DrawableBindingAdaptersTestActivity.kt

+import org.oppia.android.app.activity.ActivityComponentImpl
+import org.oppia.android.app.activity.InjectableAppCompatActivity
+
+/** Test activity for DrawableBindingAdapters. */


This reference won't work since the class isn't being imported. I suggest either using backticks, i.e.:

``DrawableBindingAdapters``

Or, fully qualify the type: [org.oppia.android.app.databinding.DrawableBindingAdapters]

(Note that you can check if a Markdown reference is correct by control clicking on the class name; if it doesn't open or has a squiggly underneath it, the reference isn't correct).

rt4914

LGTM, thanks.

rishidyno · 2022-01-28T14:59:49Z

@BenHenning PTAL

oppiabot · 2022-01-28T15:02:52Z

Unassigning @rishidyno since a re-review was requested. @rishidyno, please make sure you have addressed all review comments. Thanks!

BenHenning

Thanks @rishidyno. LGTM.

oppiabot · 2022-02-01T02:23:41Z

Unassigning @BenHenning since they have already approved the PR.

BenHenning · 2022-02-01T02:24:58Z

Everything appears resolved, so merging this.

rishidyno added 3 commits January 4, 2022 16:27

initial commit

1c2aafc

test added

315cf9e

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

3793ad7

… test-suit-for-drawablebindingadapters

rishidyno requested review from anandwana001, BenHenning and rt4914 as code owners January 4, 2022 15:14

format

bfebf1c

rishidyno added 8 commits January 11, 2022 23:12

test

9bd76dd

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

f99c38d

… test-suit-for-drawablebindingadapters

test

653587a

test

f7b13b5

test

4efe831

test

cdb5097

test

264236c

test

8c6f08d

oppiabot bot assigned BenHenning and rt4914 Jan 13, 2022

BenHenning changed the title ~~Fix #3099: Test suit for drawablebindingadapters~~ Fix #3099: Test suite for drawablebindingadapters Jan 15, 2022

BenHenning requested changes Jan 15, 2022

View reviewed changes

BenHenning assigned rishidyno and unassigned BenHenning Jan 15, 2022

rt4914 suggested changes Jan 17, 2022

View reviewed changes

rt4914 removed their assignment Jan 17, 2022

rishidyno added 2 commits January 18, 2022 14:35

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

8bbc3aa

… test-suit-for-drawablebindingadapters

few fixes

905886d

rishidyno added 3 commits January 26, 2022 19:27

commit

cf0bb9a

commit

9be4c17

commit

fe656c1

rishidyno commented Jan 26, 2022

View reviewed changes

rishidyno closed this Jan 26, 2022

rishidyno reopened this Jan 26, 2022

rishidyno closed this Jan 26, 2022

rishidyno reopened this Jan 26, 2022

rishidyno closed this Jan 26, 2022

rishidyno reopened this Jan 26, 2022

rishidyno added 2 commits January 26, 2022 19:54

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

a0f1c07

… test-suit-for-drawablebindingadapters

commit

19b8b44

rishidyno assigned rt4914 and BenHenning and unassigned rishidyno Jan 26, 2022

BenHenning approved these changes Jan 26, 2022

View reviewed changes

BenHenning assigned rishidyno and unassigned BenHenning Jan 26, 2022

rt4914 approved these changes Jan 27, 2022

View reviewed changes

rt4914 removed their assignment Jan 27, 2022

kdoc-correction

5d7683a

oppiabot bot assigned BenHenning and unassigned rishidyno Jan 28, 2022

BenHenning approved these changes Feb 1, 2022

View reviewed changes

oppiabot bot unassigned BenHenning Feb 1, 2022

BenHenning merged commit 26ab020 into oppia:develop Feb 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3099: Test suite for DrawableBindingAdapters #4077

Fix #3099: Test suite for DrawableBindingAdapters #4077

rishidyno commented Jan 4, 2022 •

edited

github-actions bot commented Jan 4, 2022

rishidyno commented Jan 4, 2022

BenHenning commented Jan 5, 2022

rishidyno commented Jan 13, 2022

BenHenning left a comment

rt4914 left a comment

rt4914 Jan 17, 2022

rishidyno Jan 18, 2022

rt4914 Jan 17, 2022

rishidyno Jan 18, 2022

rishidyno commented Jan 18, 2022 •

edited

rishidyno left a comment

rishidyno Jan 26, 2022

BenHenning left a comment

BenHenning Jan 26, 2022

rt4914 left a comment

rishidyno commented Jan 28, 2022

oppiabot bot commented Jan 28, 2022

BenHenning left a comment

oppiabot bot commented Feb 1, 2022

BenHenning commented Feb 1, 2022

Fix #3099: Test suite for DrawableBindingAdapters #4077

Fix #3099: Test suite for DrawableBindingAdapters #4077

Conversation

rishidyno commented Jan 4, 2022 • edited

Explanation

Essential Checklist

For UI-specific PRs only

github-actions bot commented Jan 4, 2022

rishidyno commented Jan 4, 2022

BenHenning commented Jan 5, 2022

rishidyno commented Jan 13, 2022

BenHenning left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Jan 17, 2022

Choose a reason for hiding this comment

rishidyno Jan 18, 2022

Choose a reason for hiding this comment

rt4914 Jan 17, 2022

Choose a reason for hiding this comment

rishidyno Jan 18, 2022

Choose a reason for hiding this comment

rishidyno commented Jan 18, 2022 • edited

rishidyno left a comment

Choose a reason for hiding this comment

rishidyno Jan 26, 2022

Choose a reason for hiding this comment

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning Jan 26, 2022

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

rishidyno commented Jan 28, 2022

oppiabot bot commented Jan 28, 2022

BenHenning left a comment

Choose a reason for hiding this comment

oppiabot bot commented Feb 1, 2022

BenHenning commented Feb 1, 2022

rishidyno commented Jan 4, 2022 •

edited

rishidyno commented Jan 18, 2022 •

edited