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.

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

Fix part of #4562: Add string consistency tool & CI check #4563

Merged
merged 32 commits into from
Sep 8, 2022

Conversation

BenHenning
Copy link
Sponsor Member

@BenHenning BenHenning commented Sep 7, 2022

Explanation

Fixes part of #4562.

This PR introduces two new scripts:

  • One for computing how many strings are not yet translated for Arabic, Brazilian Portuguese, and Swahili (as compared to the base English strings)
  • One for verifying newline consistency between the English & non-English strings (based on number of lines)

The latter was also added as one of the jobs to be run during the static checks CI workflow, and is specifically useful to ensure newlines weren't inadvertently added by translators (see the updated translation strings for an idea on how often this has happened). Note that, as part of fixing these strings, the source string for lets_get_started was updated to no longer include a newline. This seems reasonable given that we should never use newlines for spacing/styling unless it's for logical splits (like paragraphs). However, I can't verify whether actual style changes are needed since this string is used as part of the unreleased app walkthrough feature. I expect that we'll re-audit the UI of that feature when we revisit it.

This PR also includes a major performance fix for RepositoryFile that will benefit all checks that use it. Kotlin's built-in file tree walker is built to implicitly follow symlinks on the filesystem. For Bazel builds, this expands the codebase to >1M files (since directory filtering happens after the files are known). Since scripts don't actually need to follow symlinks, the utility was updated to specifically not follow them (using Java NIO's file tree walk routine) which led to an observed ~10x performance improvement. Further optimizations could be done by building our own tree-walker and filtering directories during search (which will probably be necessary if we ever do need to follow symlinks, but it probably won't be needed for a long time since the current performance is quite solid).

The former script is useful when manually auditing the translation progress using the codebase as the source of truth rather than Translatewiki. For now, it's been helpful in beta MR1 planning but longer-term it may also be useful as a badge on the repo's README.

Finally, the Kotlin stdlib dependency was updated to point to jdk8 instead of jdk7 (since a specific feature was needed from the former, and the codebase can rely on higher Java SDK versions than 7 given that the minimum Java version is 1.8).

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

N/A -- this is a developer infrastructure-only PR, except for all of the string newline fixes (which just result in a more correct UI for non-English), and the changed English string (which is inaccessible to demonstrate). Screenshots could be uploaded, but the newlines need to be removed regardless so it doesn't seem specifically useful here (plus those languages may not be obvious to verify for reviewers).

This simplifies application component management significantly and
allows individual build flavors to have their own unique module lists.
This also introduces dedicated beta & GA build flavors which is a
necessary prerequisite.

It also introduces an extra beta, alpha, and dev mode labels for the splash
screen (the latter 2 were extra) with 2 second minimum wait timers for
beta and alpha to ensure they are seen. A 5-second safety timer was
added to ensure the splash screen can always be passed even if something
goes wrong at the domain level (since there are now quite a few moving
pieces to determine the user's current onboarding state).
Tests broken due to changes to the app startup experience haven't yet
been fixed.
There's a bunch left to do here, this is mainly needed so that I can
transfer changes to a different machine.
Conflicts:
	build_flavors.bzl
	scripts/assets/test_file_exemptions.textproto
…oduce-beta-ga-notices

Conflicts:
	.github/workflows/build_tests.yml
	build_flavors.bzl
	scripts/assets/test_file_exemptions.textproto
	version.bzl
This also removes temporary debug code and TODOs, and finishes the tests
for SplashActivity.
Conflicts:
	app/src/main/java/org/oppia/android/app/application/alpha/AlphaApplicationComponent.kt
	app/src/main/java/org/oppia/android/app/application/dev/DeveloperApplicationComponent.kt
	app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AppVersionActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/completedstorylist/CompletedStoryListActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/customview/LessonThumbnailImageViewTest.kt
	app/src/sharedTest/java/org/oppia/android/app/customview/interaction/MathExpressionInteractionsViewTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/DrawableBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/ImageViewBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/StateAssemblerMarginBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/StateAssemblerPaddingBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/TextViewBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/databinding/ViewBindingAdaptersTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/DeveloperOptionsActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/DeveloperOptionsFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkChaptersCompletedFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkStoriesCompletedActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkStoriesCompletedFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkTopicsCompletedActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/MarkTopicsCompletedFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/ViewEventLogsActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/ViewEventLogsFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/forcenetworktype/ForceNetworkTypeActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/forcenetworktype/ForceNetworkTypeFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/mathexpressionparser/MathExpressionParserActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/devoptions/mathexpressionparser/MathExpressionParserFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/faq/FAQListFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/faq/FAQSingleActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/faq/FaqListActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/help/HelpActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/help/HelpFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/home/HomeActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/home/RecentlyPlayedFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/home/TopicSummaryViewModelTest.kt
	app/src/sharedTest/java/org/oppia/android/app/home/WelcomeViewModelTest.kt
	app/src/sharedTest/java/org/oppia/android/app/home/promotedlist/PromotedStoryListViewModelTest.kt
	app/src/sharedTest/java/org/oppia/android/app/home/promotedlist/PromotedStoryViewModelTest.kt
	app/src/sharedTest/java/org/oppia/android/app/mydownloads/MyDownloadsActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/mydownloads/MyDownloadsFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/onboarding/OnboardingActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/onboarding/OnboardingFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicListActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/AppLanguageActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/AppLanguageFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/AudioLanguageActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/AudioLanguageFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/OptionsActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/OptionsFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/ReadingTextSizeActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/options/ReadingTextSizeFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/parser/CustomBulletSpanTest.kt
	app/src/sharedTest/java/org/oppia/android/app/parser/HtmlParserTest.kt
	app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profile/AdminAuthActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profile/AdminPinActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profile/PinPasswordActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profile/ProfileChooserFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profileprogress/ProfilePictureActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profileprogress/ProfileProgressActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/profileprogress/ProfileProgressFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/recyclerview/BindableAdapterTest.kt
	app/src/sharedTest/java/org/oppia/android/app/resumelesson/ResumeLessonActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileEditFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileListActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileListFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileRenameActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileRenameFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileResetPinActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/settings/profile/ProfileResetPinFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/story/StoryActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/DragDropTestActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/ImageRegionSelectionInteractionViewTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/InputInteractionViewTestActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/NavigationDrawerActivityDebugTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/NavigationDrawerActivityProdTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/TestFontScaleConfigurationUtilActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/testing/TopicTestActivityForStoryTest.kt
	app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseListActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseListFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseTextViewerActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/thirdparty/LicenseTextViewerFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/thirdparty/ThirdPartyDependencyListActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/thirdparty/ThirdPartyDependencyListFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/TopicActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/TopicFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/conceptcard/ConceptCardFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/info/TopicInfoFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/practice/TopicPracticeFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/revision/TopicRevisionFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/utility/RatioExtensionsTest.kt
	app/src/sharedTest/java/org/oppia/android/app/walkthrough/WalkthroughActivityTest.kt
	app/src/sharedTest/java/org/oppia/android/app/walkthrough/WalkthroughFinalFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/walkthrough/WalkthroughTopicListFragmentTest.kt
	app/src/sharedTest/java/org/oppia/android/app/walkthrough/WalkthroughWelcomeFragmentTest.kt
	app/src/test/java/org/oppia/android/app/activity/ActivityIntentFactoriesTest.kt
	app/src/test/java/org/oppia/android/app/home/HomeActivityLocalTest.kt
	app/src/test/java/org/oppia/android/app/parser/FractionParsingUiErrorTest.kt
	app/src/test/java/org/oppia/android/app/parser/StringToRatioParserTest.kt
	app/src/test/java/org/oppia/android/app/player/exploration/ExplorationActivityLocalTest.kt
	app/src/test/java/org/oppia/android/app/player/state/StateFragmentLocalTest.kt
	app/src/test/java/org/oppia/android/app/profile/ProfileChooserFragmentLocalTest.kt
	app/src/test/java/org/oppia/android/app/story/StoryActivityLocalTest.kt
	app/src/test/java/org/oppia/android/app/testing/CompletedStoryListSpanTest.kt
	app/src/test/java/org/oppia/android/app/testing/HomeSpanTest.kt
	app/src/test/java/org/oppia/android/app/testing/OngoingTopicListSpanTest.kt
	app/src/test/java/org/oppia/android/app/testing/PlatformParameterIntegrationTest.kt
	app/src/test/java/org/oppia/android/app/testing/ProfileChooserSpanTest.kt
	app/src/test/java/org/oppia/android/app/testing/ProfileProgressSpanCountTest.kt
	app/src/test/java/org/oppia/android/app/testing/RecentlyPlayedSpanTest.kt
	app/src/test/java/org/oppia/android/app/testing/TopicRevisionSpanTest.kt
	app/src/test/java/org/oppia/android/app/testing/activity/TestActivityTest.kt
	app/src/test/java/org/oppia/android/app/testing/administratorcontrols/AdministratorControlsFragmentTest.kt
	app/src/test/java/org/oppia/android/app/testing/options/OptionsFragmentTest.kt
	app/src/test/java/org/oppia/android/app/testing/player/split/PlayerSplitScreenTest.kt
	app/src/test/java/org/oppia/android/app/testing/player/state/StateFragmentAccessibilityTest.kt
	app/src/test/java/org/oppia/android/app/topic/info/TopicInfoFragmentLocalTest.kt
	app/src/test/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentLocalTest.kt
	app/src/test/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityLocalTest.kt
	app/src/test/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityLocalTest.kt
	app/src/test/java/org/oppia/android/app/translation/AppLanguageResourceHandlerTest.kt
	app/src/test/java/org/oppia/android/app/translation/AppLanguageWatcherMixinTest.kt
	app/src/test/java/org/oppia/android/app/utility/math/MathExpressionAccessibilityUtilTest.kt
	instrumentation/src/java/org/oppia/android/instrumentation/application/TestApplicationComponent.kt
	testing/src/test/java/org/oppia/android/testing/junit/InitializeDefaultLocaleRuleCustomContextTest.kt
	testing/src/test/java/org/oppia/android/testing/junit/InitializeDefaultLocaleRuleOmissionTest.kt
	testing/src/test/java/org/oppia/android/testing/junit/InitializeDefaultLocaleRuleTest.kt
Also, fixes major performance issue with all file-based CI checks.
Also, fix reporting in new validation check script.
@BenHenning BenHenning changed the title Fix part of #4562: Add string consistency tool & CI check Fix part of #4562: Add string consistency tool & CI check [Blocked on #4417] Sep 7, 2022
@BenHenning BenHenning changed the base branch from develop to introduce-beta-ga-notices September 7, 2022 09:45
@BenHenning BenHenning changed the title Fix part of #4562: Add string consistency tool & CI check [Blocked on #4417] Fix part of #4562: Add string consistency tool & CI check [Blocked: #4417] Sep 7, 2022
@BenHenning BenHenning self-assigned this Sep 8, 2022
@BenHenning BenHenning marked this pull request as ready for review September 8, 2022 04:46
@BenHenning
Copy link
Sponsor Member Author

BenHenning commented Sep 8, 2022

This should be ready to merge once #4417 is merged and this PR brought up-to-date with develop (I self reviewed it and didn't find anything to change).

Base automatically changed from introduce-beta-ga-notices to develop September 8, 2022 05:40
@BenHenning BenHenning requested review from rt4914 and a team as code owners September 8, 2022 05:40
@BenHenning BenHenning changed the title Fix part of #4562: Add string consistency tool & CI check [Blocked: #4417] Fix part of #4562: Add string consistency tool & CI check Sep 8, 2022
@BenHenning
Copy link
Sponsor Member Author

Since this PR is part of broader urgent work for the Beta MR1 release, I'm force-merging this without review. I've self-reviewed it and found no major issues. Furthermore, #4567 is tracking ensuring that this does get reviewed by someone else later after it's been merged.

@BenHenning BenHenning merged commit 34f9b16 into develop Sep 8, 2022
@BenHenning BenHenning deleted the add-strings-checks-and-fix-strings branch September 8, 2022 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant