Fix #3103: Segregate OptionsFragment tests

[Sparsh1212]

Explanation

Fixes #3103: Tests Segregation in OptionsFragmentTest

Espresso Test results snapshots:

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.

[Sparsh1212]

@rt4914
I have completed the Part-A of the issue which involves:

1. Write a test in OptionsFragmentTest to check that the correct default Language is displayed in the recycler view of the Options file. (Repeat this a total of three times to check for App Language, Audio Language, and ReadingTextSize)
2. Repeat the above three tests for a configuration change.
3. Write a test for clicking an item in the Options Recycler View and check if it is sending the correct intent based on the default Language (Repeat this three times to check for App Language, Audio Language, and ReadingTextSize)
4. Also, repeat the above three tests (Point number three) for a configuration change.

Will proceed to Part-B of the issue after getting an initial pass on Part-A.

[rt4914]

The tests are failing on Roboelectric.
Each test should pass on Roboelectric as well as Espresso. Also make sure that CI checks also pass.

[Sparsh1212]

The tests are failing on Roboelectric.
Each test should pass on Roboelectric as well as Espresso. Also make sure that CI checks also pass.

Apologies! will remember to check each of them before making a commit.
Update: All the checks and tests are now passing. I have updated the PR with the requested changes. PTAL.

[rt4914]

@Sparsh1212 I will review this tomorrow but meanwhile you can move to part B too.

[Sparsh1212]

@rt4914
Here's the update about the Part-B work that I have initiated:

• All the previous tests in AppLanguageFragmentTest are removed and these four new tests are added keeping in mind the ideal practice (i.e not to launch the other Activity and only use the Test File Activity):

  1. testAppLanguage_selectedLanguageIsEnglish()
  2. testAppLanguage_changeConfiguration_selectedLanguageIsEnglish()
  3. testAppLanguage_changeLanguageToFrench_selectedLanguageIsFrench()
  4. testAppLanguage_changeLanguageToFrench_changeConfiguration_selectedLanguageIsFrench()

• I also tried adding this test:

   @Test 
   fun testApp_Language_pressBackBtn_SendsBackCorrectInfo(){
      launch<AppLanguageActivity>(createAppLanguageActivityIntent("English")).use {
          testCoroutineDispatchers.runCurrent()
          pressBack()
          intended(hasExtra(MESSAGE_APP_LANGUAGE_ARGUMENT_KEY, "English"))
    }
   }

But it is failing in Roboelectric with the error that 0 intends matched.
It looks like the Back press is getting ignored totally.
I also tried many other things to click the back button or to run the onBackPress function but nothing seems to work. Can you please take a look at this and guide me on what I am doing wrong here or if this approach isn't correct then how can we achieve checking that it is sending back the correct info on the back press through another approach.

• I have not made any changes to ReadingTextSizeFragmentTest because currently there is a PR (Fixes #2628: Reading Text Size[A11y] #2929) that will propose some changes in this file, so I think it will be better to wait to rewrite this file until those changes are merged.

• Will make all the similar changes for AudioLanguageFragmentTest once I get a pass on the above changes and the getting back correct info on the back press problem is resolved.

Thanks!

[rt4914]

@Test 
   fun testApp_Language_pressBackBtn_SendsBackCorrectInfo(){
      launch<AppLanguageActivity>(createAppLanguageActivityIntent("English")).use {
          testCoroutineDispatchers.runCurrent()
          pressBack()
          intended(hasExtra(MESSAGE_APP_LANGUAGE_ARGUMENT_KEY, "English"))
    }
   }

@Sparsh1212 Have you checked ShadowActivity? https://stackoverflow.com/a/8991026

[anandwana001]

regarding the testApp_Language_pressBackBtn_SendsBackCorrectInfo , we cannot use ShadowActivity, this will make this test robolectric only.

The reason, I am seeing here why the test fails is:

override fun onBackPressed() {
    val message = appLanguageActivityPresenter.getLanguageSelected()
    val intent = Intent()
    intent.putExtra(MESSAGE_APP_LANGUAGE_ARGUMENT_KEY, message)
    setResult(REQUEST_CODE_APP_LANGUAGE, intent)
    finish()
  }

on back press, the activity gets finish, so test is basically trying to test after activity gets finished.
I don't think this test will pass on espresso also. This test seems like to come under e2e test.

Also, add all the espresso test result screenshots in PR description.

[Sparsh1212]

regarding the testApp_Language_pressBackBtn_SendsBackCorrectInfo , we cannot use ShadowActivity, this will make this test robolectric only.

The reason, I am seeing here why the test fails is:

override fun onBackPressed() {
    val message = appLanguageActivityPresenter.getLanguageSelected()
    val intent = Intent()
    intent.putExtra(MESSAGE_APP_LANGUAGE_ARGUMENT_KEY, message)
    setResult(REQUEST_CODE_APP_LANGUAGE, intent)
    finish()
  }

on back press, the activity gets finish, so test is basically trying to test after activity gets finished.
I don't think this test will pass on espresso also. This test seems like to come under e2e test.

Also, add all the espresso test result screenshots in PR description.

Thanks, will update and make all the suggested changes.

[rt4914]

Un-assigning until @anandwana001 approves it.

[Sparsh1212]

Thanks @Sparsh1212
shifting to sharedTest is good.

Need tablet test result screenshot. Please run these tests on tablet config and see if they are passing correctly.

Added.

[anandwana001]

LGTM, Thanks @Sparsh1212

[rt4914]

@Sparsh1212 Replied to my one of the open comments.

[rt4914]

Actually you have removed it from test file. But instead what we want it we should either use only

const val MESSAGE_READING_TEXT_SIZE_ARGUMENT_KEY = "OptionsFragment.message_reading_text_size"
const val MESSAGE_APP_LANGUAGE_ARGUMENT_KEY = "OptionsFragment.message_app_language"
const val MESSAGE_AUDIO_LANGUAGE_ARGUMENT_KEY = "OptionsFragment.message_audio_language"

in tests and production code

or we should use only

const val KEY_READING_TEXT_SIZE_PREFERENCE_SUMMARY_VALUE =
      "READING_TEXT_SIZE_PREFERENCE_SUMMARY_VALUE"
    const val APP_LANGUAGE_PREFERENCE_SUMMARY_VALUE_EXTRA_KEY =
      "AppLanguageActivity.app_language_preference_summary_value"
    const val KEY_AUDIO_LANGUAGE_PREFERENCE_SUMMARY_VALUE =
      "AUDIO_LANGUAGE_PREFERENCE_SUMMARY_VALUE"

Currently you are using the above for the app layer code and the below for test cases which is incorrect. We should use only one of these sets in both code.

[rt4914]

LGTM, thanks.
Unfortunately we cannot merge this until #3157 is fixed.

[Sparsh1212]

@rt4914 I have taken the pull from the latest develop branch. The checks are also passing. Looks like we can merge this now :)