Fixes #1882: NumericInput tests are shifted to a new folder and their package is updated to match the directory structure. #1919

soamOne · 2020-09-29T19:54:13Z

Explanation

Fixes #1882: In this PR we shift the numericinput test files to a new folder called numericinput. The package in the files is
updated to match the directory structure of the files. The Bazel files are also updated
with the correct directory structure.

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.

soamOne · 2020-09-29T20:00:44Z

@BenHenning @rt4914 PTAL.

soamOne · 2020-09-30T02:53:15Z

Hey @rt4914 @BenHenning, Why are the Kotlin tests failing due to import statements, even though the last PR passed these tests?

aggarwalpulkit596 · 2020-09-30T02:54:08Z

@soamOne the lints are failing. Could you fix those ? Also you can check locally by running ktlint --android

soamOne · 2020-09-30T03:18:33Z

Hey @rt4914 @BenHenning, Why are the Kotlin tests failing due to import statements, even though the last PR passed these tests?

@aggarwalpulkit596 can you please help with this?

anandwana001 · 2020-09-30T04:13:29Z

Hi @soamOne

Ktlint is failing because you have a few unused imports in these files. I had mentioned the files and the line number, you can check and remove these lines which are unused. Also, remember while removing that the imports should be in lexicographical order else ktlint fail again. To automate this lexicographical order you can reformat the code from the menu option.

numericinput/NumericInputEqualsRuleClassifierProviderTest.kt:13:1: Unnecessary import
numericinput/NumericInputIsGreaterThanOrEqualToRuleClassifierProviderTest.kt:13:1: Unnecessary import
numericinput/NumericInputIsGreaterThanRuleClassifierProviderTest.kt:13:1: Unnecessary import
numericinput/NumericInputIsInclusivelyBetweenRuleClassifierProviderTest.kt:13:1: Unnecessary import
numericinput/NumericInputIsLessThanOrEqualToRuleClassifierProviderTest.kt:13:1: Unnecessary import
numericinput/NumericInputIsLessThanRuleClassifierProviderTest.kt:13:1: Unnecessary import

You can simply remove them using optimize imports. Also, for the future works, I suggest you set macro as per description here https://github.com/oppia/oppia-android/wiki/Ktlint-Guide#macros

This will reformat your code and also optimize the imports, so there will be few chances of ktlint fails.
You can also run ktlint on your local machine as per the instruction here https://github.com/oppia/oppia-android/wiki/Ktlint-Guide#commands

BenHenning · 2020-09-30T05:10:06Z

You can also run the ktlint tool on the command line with '-F' to automatically fix linter errors, though it's good practice to make sure that you know how to manually fix them as well. :)

BenHenning

Thanks @soamOne! The changes generally look good, but please fix the ktlint errors and I'll take one more pass on this PR.

prayutsu · 2020-09-30T09:08:38Z

app/src/main/java/org/oppia/android/app/profile/ProfileInputView.kt

-        R.styleable.ProfileInputView_isPasswordInput,
-        /** defVal= */ false
-      )
+          R.styleable.ProfileInputView_isPasswordInput,


@soamOne You have changed indentation here, fixing that would solve the ktlint error

prayutsu · 2020-09-30T09:08:54Z

app/src/main/java/org/oppia/android/app/recyclerview/StartSnapHelper.kt

@@ -68,8 +68,8 @@ class StartSnapHelper : LinearSnapHelper() {
      return if (helper.getDecoratedEnd(child) >=
        helper.getDecoratedMeasurement(child) / 2 &&
        helper.getDecoratedEnd(
-          child
-        ) > 0
+            child


Same applies here

This reverts commit 5b4a0c9.

soamOne · 2020-09-30T10:10:27Z

Thanks a lot @rt4914 @aggarwalpulkit596 @anandwana001 @prayutsu !!
@BenHenning can you please take a look now, thanks!

anandwana001

@soamOne thanks
LGTM, one last thing.

anandwana001 · 2020-09-30T10:11:50Z

app/src/main/java/org/oppia/android/app/profile/ProfileInputView.kt

@@ -61,7 +61,8 @@ class ProfileInputView @JvmOverloads constructor(
    if (
      attributes.getBoolean(
        R.styleable.ProfileInputView_isPasswordInput,
-        /** defVal= */ false
+        /** defVal= */
+        false


One last thing from my side, could you make these in one line?

Sure, will do!

This doesn't appear done yet--please fix.

prayutsu

@soamOne Looks good to me, thanks!

anandwana001

LGTM, Thanks @soamOne

This reverts commit 37a57fd.

BenHenning

Thanks @soamOne! The PR LGTM, but please fix the extra change in ProfileInputView.kt.

BenHenning · 2020-09-30T19:46:11Z

app/src/main/java/org/oppia/android/app/profile/ProfileInputView.kt

@@ -61,7 +61,8 @@ class ProfileInputView @JvmOverloads constructor(
    if (
      attributes.getBoolean(
        R.styleable.ProfileInputView_isPasswordInput,
-        /** defVal= */ false
+        /** defVal= */
+        false


This doesn't appear done yet--please fix.

soamOne · 2020-09-30T20:28:04Z

@BenHenning i tried fixing that but for some reason the checks were failing, so i reverted it.

BenHenning · 2020-09-30T21:45:08Z

@soamOne what specific check was failing? It's important to make sure we don't undo work that we previously mentioned was completed, and to be specific about why changes can't be done otherwise reviewers don't have sufficient context to review the code. :)

soamOne · 2020-10-01T04:23:30Z

@BenHenning I get your point :)
Actually the app module Robolectric checks were failing with a java.lang.RuntimeException.

BenHenning · 2020-10-01T05:10:32Z

Thanks @soamOne! Feel free to ask for help if needed (e.g. if an error that you encounter doesn't make sense).

BenHenning · 2020-10-01T05:14:56Z

Also, when you encounter an exception you should provide:

The steps you did to trigger it
The stack trace you’re seeing
Your initial thoughts of what’s going wrong and what you’ve tried to fix

Without this information, it’s hard for others to help since they don’t have as much context as you on the inner workings of your PR.

soamOne · 2020-10-01T07:19:50Z

@BenHenning can you PTAL now, thanks :)

BenHenning

This looks good, thanks @soamOne!

BenHenning · 2020-10-01T20:38:38Z

Going ahead and merging this since everything is looking good.

soamOne requested a review from BenHenning as a code owner September 29, 2020 19:54

anandwana001 assigned BenHenning and soamOne and unassigned BenHenning Sep 30, 2020

BenHenning requested changes Sep 30, 2020

View reviewed changes

prayutsu reviewed Sep 30, 2020

View reviewed changes

soamOne added 6 commits September 30, 2020 15:25

fixed the numericinput tests folder

47181ca

updated the package of numeric input tests in BUILD.bazel

a0f55a8

fixed imports in numeric input tests

f1d2501

Revert "fixed imports in numeric input tests"

ebd45bc

This reverts commit 5b4a0c9.

fixed ktlint errors

3afa2f2

fixed indentation

c2a7608

anandwana001 suggested changes Sep 30, 2020

View reviewed changes

prayutsu self-requested a review September 30, 2020 10:13

fixed indentation

37a57fd

soamOne requested review from anandwana001 and BenHenning September 30, 2020 10:18

prayutsu approved these changes Sep 30, 2020

View reviewed changes

anandwana001 approved these changes Sep 30, 2020

View reviewed changes

anandwana001 assigned BenHenning and unassigned soamOne Sep 30, 2020

Revert "fixed indentation"

2f5602c

This reverts commit 37a57fd.

BenHenning requested changes Sep 30, 2020

View reviewed changes

BenHenning assigned soamOne and unassigned BenHenning Sep 30, 2020

fixed indentation

68122d6

soamOne requested a review from BenHenning October 1, 2020 19:18

BenHenning approved these changes Oct 1, 2020

View reviewed changes

BenHenning merged commit de89de7 into oppia:develop Oct 1, 2020

soamOne deleted the numeric-input-tests branch October 1, 2020 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #1882: NumericInput tests are shifted to a new folder and their package is updated to match the directory structure. #1919

Fixes #1882: NumericInput tests are shifted to a new folder and their package is updated to match the directory structure. #1919

soamOne commented Sep 29, 2020

soamOne commented Sep 29, 2020

soamOne commented Sep 30, 2020

aggarwalpulkit596 commented Sep 30, 2020

soamOne commented Sep 30, 2020

anandwana001 commented Sep 30, 2020 •

edited

Loading

BenHenning commented Sep 30, 2020

BenHenning left a comment

prayutsu Sep 30, 2020

prayutsu Sep 30, 2020

soamOne commented Sep 30, 2020

anandwana001 left a comment •

edited

Loading

anandwana001 Sep 30, 2020

soamOne Sep 30, 2020

soamOne Sep 30, 2020

BenHenning Sep 30, 2020

prayutsu left a comment •

edited

Loading

anandwana001 left a comment

BenHenning left a comment

BenHenning Sep 30, 2020

soamOne commented Sep 30, 2020

BenHenning commented Sep 30, 2020

soamOne commented Oct 1, 2020 •

edited

Loading

BenHenning commented Oct 1, 2020

BenHenning commented Oct 1, 2020 •

edited

Loading

soamOne commented Oct 1, 2020

BenHenning left a comment

BenHenning commented Oct 1, 2020

Fixes #1882: NumericInput tests are shifted to a new folder and their package is updated to match the directory structure. #1919

Fixes #1882: NumericInput tests are shifted to a new folder and their package is updated to match the directory structure. #1919

Conversation

soamOne commented Sep 29, 2020

Explanation

Checklist

soamOne commented Sep 29, 2020

soamOne commented Sep 30, 2020

aggarwalpulkit596 commented Sep 30, 2020

soamOne commented Sep 30, 2020

anandwana001 commented Sep 30, 2020 • edited Loading

BenHenning commented Sep 30, 2020

BenHenning left a comment

Choose a reason for hiding this comment

prayutsu Sep 30, 2020

Choose a reason for hiding this comment

prayutsu Sep 30, 2020

Choose a reason for hiding this comment

soamOne commented Sep 30, 2020

anandwana001 left a comment • edited Loading

Choose a reason for hiding this comment

anandwana001 Sep 30, 2020

Choose a reason for hiding this comment

soamOne Sep 30, 2020

Choose a reason for hiding this comment

soamOne Sep 30, 2020

Choose a reason for hiding this comment

BenHenning Sep 30, 2020

Choose a reason for hiding this comment

prayutsu left a comment • edited Loading

Choose a reason for hiding this comment

anandwana001 left a comment

Choose a reason for hiding this comment

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning Sep 30, 2020

Choose a reason for hiding this comment

soamOne commented Sep 30, 2020

BenHenning commented Sep 30, 2020

soamOne commented Oct 1, 2020 • edited Loading

BenHenning commented Oct 1, 2020

BenHenning commented Oct 1, 2020 • edited Loading

soamOne commented Oct 1, 2020

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning commented Oct 1, 2020

anandwana001 commented Sep 30, 2020 •

edited

Loading

anandwana001 left a comment •

edited

Loading

prayutsu left a comment •

edited

Loading

soamOne commented Oct 1, 2020 •

edited

Loading

BenHenning commented Oct 1, 2020 •

edited

Loading