From 6610306487b68ce4cc3e905419cf9b8f7275be8d Mon Sep 17 00:00:00 2001 From: Sparsh Agrawal <55937724+Sparsh1212@users.noreply.github.com> Date: Thu, 29 Jul 2021 23:52:55 +0530 Subject: [PATCH] Fix #3300: Add check for accessibility label for activities (#3352) * Fix #3290: Add support for generic regex pattern matching * Apply review suggestions for proto location * Delete main directory * Implment script logic and tests * CI setup part 1 * syntax fix in yaml file * import nits in dummy file * Use regex patterns from script in script test * Make PR suggestions into effect * Make PR suggestions into effect * Make PR suggestions into effect * Improve naming of script_assets variables * Make PR suggestions into effect * Make PR suggestions * Make PR suggestions * Revamp testing approach * Fix #3291: Add check for XML syntax correctness * nit fixes * Introduce XML syntax check in static checks workflow * Fix #3292: Add check for test files presence for prod files * Make nit suggestions * Make nit suggestions * Make nit suggestions * Fix #3300: Add check for accessibility labels for activities * Add CI check for label presence for activities * Add ScriptConstants file to exemptions test list * Test if: always() * Revert "Test if: always()" This reverts commit b72e419b * Add if: always() * Add if: always() * Add if: always() * Apply review suggestions on PR * Make review suggestions from regex pattern checks * Implement review suggestions based on #3340 * Refactor PR as per feedback recieved * Implement feedback suggestions * Implement review suggestions * Implement review suggestions * Implement review suggestions * Implement review suggestions * nits * Nit fixes * Nit fixes * Nit fixes * Implement feedback suggestions as per #3340 * nit fix * nit fix * nit fix * nit fix * Review suggestions part 1 * Implement review suggestions part 2 * nit fixes * update static_checks * add test to repository file * nit fix * nit fix * nit fix * Do review suggestions * Implement review suggestions * Implement review suggestions * nit fix * Implement review suggestions * Add Ben as a codeowner for the ScriptExemptions file * bazel files nit fix * Introduce a library for the regex assets * Make directory structure same as that of #3374 * Make directory structure similar to #3374 * add new files to test file script exemptions and nit fixes * diable ktlint max-line-length * disable ktlint-max-line * disable ktlint max-length * remove testonly attribute from tests * add new files to script exemptions * nit fix * nit fix * nit fix * Apply review suggestions * ktlint fix * nit fix * Apply nit fixes * Apply nit fixes * Remove script constants * nit fix * add todo * add todo * nit fixes * add test * add testOnly * nit fixes * nit fix * nit fixes * nit fixes * nit fixes * nit fixes * fix textproto file * add exemptions to textproto * sort failures lexicographically * add new files to exemption list * nit fixes * nit fixes * add kdocs * nit fix * nit fix * Trigger CI failure exclusively * Revert CI check failure * nit fix * nit fix * nit fixes * nit fixes * nit fixes * Add redundant exemptions check * rectify static_checks * Remove duplicate proto libraries * nit fixes * add activity to exemption * nit fixes * nit fixes * nit fixes * update branch --- .github/workflows/static_checks.yml | 5 + scripts/BUILD.bazel | 16 + .../accessibility_label_exemptions.textproto | 28 ++ scripts/script_assets.bzl | 29 +- .../scripts/label/AccessibilityLabelCheck.kt | 203 ++++++++ .../oppia/android/scripts/label/BUILD.bazel | 17 + .../oppia/android/scripts/proto/BUILD.bazel | 6 +- .../scripts/proto/script_exemptions.proto | 16 + .../android/scripts/testfile/BUILD.bazel | 2 +- .../label/AccessibilityLabelCheckTest.kt | 474 ++++++++++++++++++ .../oppia/android/scripts/label/BUILD.bazel | 17 + 11 files changed, 807 insertions(+), 6 deletions(-) create mode 100644 scripts/assets/accessibility_label_exemptions.textproto create mode 100644 scripts/src/java/org/oppia/android/scripts/label/AccessibilityLabelCheck.kt create mode 100644 scripts/src/java/org/oppia/android/scripts/label/BUILD.bazel create mode 100644 scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt create mode 100644 scripts/src/javatests/org/oppia/android/scripts/label/BUILD.bazel diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index 6bf891f7cdf..6de4b869073 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -118,3 +118,8 @@ jobs: if: always() run: | bazel run //scripts:test_file_check -- $(pwd) + + - name: Accessibility label Check + if: always() + run: | + bazel run //scripts:accessibility_label_check -- $(pwd) scripts/assets/accessibility_label_exemptions.pb app/src/main/AndroidManifest.xml diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index 9d24d49ce24..157659e7ea7 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -5,6 +5,7 @@ Kotlin-based scripts to help developers or perform continuous integration tasks. load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_binary", "kt_jvm_library") load( "//scripts:script_assets.bzl", + "generate_accessibility_label_assets_list_from_text_protos", "generate_maven_assets_list_from_text_protos", "generate_regex_assets_list_from_text_protos", "generate_test_file_assets_list_from_text_protos", @@ -115,3 +116,18 @@ kt_jvm_binary( main_class = "org.oppia.android.scripts.maven.GenerateMavenDependenciesListKt", runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/maven:generate_maven_dependencies_list_lib"], ) + +ACCESSIBILITY_LABEL_EXEMPTION_ASSETS = generate_accessibility_label_assets_list_from_text_protos( + name = "accessibility_label_exemption_assets", + accessibility_label_exemptions_name = ["accessibility_label_exemptions"], +) + +kt_jvm_binary( + name = "accessibility_label_check", + testonly = True, + data = ACCESSIBILITY_LABEL_EXEMPTION_ASSETS, + main_class = "org.oppia.android.scripts.label.AccessibilityLabelCheckKt", + runtime_deps = [ + "//scripts/src/java/org/oppia/android/scripts/label:accessibility_label_check_lib", + ], +) diff --git a/scripts/assets/accessibility_label_exemptions.textproto b/scripts/assets/accessibility_label_exemptions.textproto new file mode 100644 index 00000000000..0221ecdda16 --- /dev/null +++ b/scripts/assets/accessibility_label_exemptions.textproto @@ -0,0 +1,28 @@ +exempted_activity: "app/src/main/java/org/oppia/android/app/devoptions/marktopicscompleted/testing/MarkTopicsCompletedTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/home/HomeActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/mydownloads/MyDownloadsActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/player/state/testing/StateFragmentTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/profileprogress/ProfilePictureActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/settings/profile/ProfileEditActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/splash/SplashActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/AudioFragmentTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/BindableAdapterTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/DragDropTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/ExplorationInjectionActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/ExplorationTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/HomeFragmentTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/HomeTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/HtmlParserTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/ImageRegionSelectionTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/InputInteractionViewTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/LessonThumbnailImageViewTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/NavigationDrawerTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/ProfileChooserFragmentTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/SplashTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/TestFontScaleConfigurationUtilActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/TopicRevisionTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/TopicTestActivity" +exempted_activity: "app/src/main/java/org/oppia/android/app/testing/TopicTestActivityForStory" +exempted_activity: "app/src/main/java/org/oppia/android/app/walkthrough/WalkthroughActivity" diff --git a/scripts/script_assets.bzl b/scripts/script_assets.bzl index 4095e83cc52..ba53566b084 100644 --- a/scripts/script_assets.bzl +++ b/scripts/script_assets.bzl @@ -47,7 +47,7 @@ def generate_test_file_assets_list_from_text_protos( Args: name: str. The name of this generation instance. This will be a prefix for derived targets. - test_file_exemptions_name: list of str. The list of test file exemptions file names. + test_file_exemptions_name: list of str. The list of test file exemptions file name. Returns: list of str. The list of new proto binary asset files that were generated. @@ -55,7 +55,7 @@ def generate_test_file_assets_list_from_text_protos( return generate_proto_binary_assets( name = name, names = test_file_exemptions_name, - proto_dep_name = "test_file_exemptions", + proto_dep_name = "script_exemptions", proto_type_name = "TestFileExemptions", name_prefix = name, asset_dir = "assets", @@ -87,3 +87,28 @@ def generate_maven_assets_list_from_text_protos( proto_dep_bazel_target_prefix = "//scripts/src/java/org/oppia/android/scripts/proto", proto_package = "proto", ) + +def generate_accessibility_label_assets_list_from_text_protos( + name, + accessibility_label_exemptions_name): + """ + Converts a single list of text proto assets to binary. + + Args: + name: str. The name of this generation instance. This will be a prefix for derived targets. + accessibility_label_exemptions_name: list of str. The list of accessibility label exemptions + file name. + + Returns: + list of str. The list of new proto binary asset files that were generated. + """ + return generate_proto_binary_assets( + name = name, + names = accessibility_label_exemptions_name, + proto_dep_name = "script_exemptions", + proto_type_name = "AccessibilityLabelExemptions", + name_prefix = name, + asset_dir = "assets", + proto_dep_bazel_target_prefix = "//scripts/src/java/org/oppia/android/scripts/proto", + proto_package = "proto", + ) diff --git a/scripts/src/java/org/oppia/android/scripts/label/AccessibilityLabelCheck.kt b/scripts/src/java/org/oppia/android/scripts/label/AccessibilityLabelCheck.kt new file mode 100644 index 00000000000..70913f93636 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/label/AccessibilityLabelCheck.kt @@ -0,0 +1,203 @@ +package org.oppia.android.scripts.label + +import org.oppia.android.scripts.proto.AccessibilityLabelExemptions +import org.w3c.dom.Node +import org.w3c.dom.NodeList +import java.io.File +import java.io.FileInputStream +import javax.xml.parsers.DocumentBuilderFactory + +/** + * Script for ensuring that all the Activities in the codebase are defined with accessibility + * labels. + * + * Usage: + * bazel run //scripts:accessibility_label_check -- + * [paths to manifest files...] + * + * Arguments: + * - path_to_directory_root: directory path to the root of the Oppia Android repository. + * - path_to_proto_binary: relative path to the exemption .pb file. + * - paths to manifest files: paths leading to the manifest files. + * + * Example: + * bazel run //scripts:accessibility_label_check -- $(pwd) app/src/main/AndroidManifest.xml + * scripts/assets/accessibility_label_exemptions.pb + */ +fun main(vararg args: String) { + val repoPath = "${args[0]}/" + + val pathToProtoBinary = args[1] + + val accessibilityLabelExemptionTextProtoFilePath = "scripts/assets/accessibility_label_exemptions" + + val accessibilityLabelExemptionList = + loadAccessibilityLabelExemptionsProto(pathToProtoBinary).getExemptedActivityList() + + val manifestPaths = args.drop(2) + + val activityPathPrefix = "app/src/main/java/" + + val builderFactory = DocumentBuilderFactory.newInstance() + + val repoRoot = File(repoPath) + + val missingAccessibilityLabelActivities = manifestPaths.flatMap { relativePath -> + val file = File(repoRoot, relativePath) + val docBuilder = builderFactory.newDocumentBuilder() + val doc = docBuilder.parse(file) + // Normalisation results in the removal of redundancies such as whitespaces, line breaks and + // comments. + doc.getDocumentElement().normalize() + val packageName = doc.getDocumentElement().getAttribute("package") + return@flatMap doc.getElementsByTagName("activity").toListOfNodes().mapNotNull { activityNode -> + computeFailureActivityPath( + activityNode = activityNode, + activityPathPrefix = activityPathPrefix, + packageName = packageName + ) + } + } + + val redundantExemptions = accessibilityLabelExemptionList - missingAccessibilityLabelActivities + + val failureActivitiesAfterExemption = missingAccessibilityLabelActivities - + accessibilityLabelExemptionList + + logRedundantExemptions(redundantExemptions, accessibilityLabelExemptionTextProtoFilePath) + + logFailures(failureActivitiesAfterExemption, accessibilityLabelExemptionTextProtoFilePath) + + if (failureActivitiesAfterExemption.isNotEmpty() || redundantExemptions.isNotEmpty()) { + throw Exception("ACCESSIBILITY LABEL CHECK FAILED") + } else { + println("ACCESSIBILITY LABEL CHECK PASSED") + } +} + +/** + * Computes path of the activity which fails the accesssibility label check. + * + * @param activityNode the activity node + * @param activityPathPrefix the path prefix for the activities + * @param packageName the package attribute value of the manifest element + * @return path of the failing activity relative to the root repository. This returns null if the + * activity has an accessibility label present. + */ +private fun computeFailureActivityPath( + activityNode: Node, + activityPathPrefix: String, + packageName: String +): String? { + val attributesList = activityNode.getAttributes() + val activityName = attributesList.getNamedItem("android:name").getNodeValue() + val activityPath = computeActivityPathFromName( + activityPathPrefix = activityPathPrefix, + activityName = activityName, + packageName = packageName + ) + if (attributesList.getNamedItem("android:label") != null) { + return null + } + return activityPath +} + +/** + * Computes the activity path from the name attribute value of the activity element. + * + * @param activityPathPrefix the path prefix for the activities + * @param activityName the name attribute value of the activity element + * @param packageName the package attribute value of the manifest element + * @return the activity path relative to the root repository + */ +private fun computeActivityPathFromName( + activityPathPrefix: String, + activityName: String, + packageName: String +): String { + if (activityName.startsWith(".")) { + return activityPathPrefix + (packageName + activityName).replace(".", "/") + } else { + return activityPathPrefix + activityName.replace(".", "/") + } +} + +/** + * Converts [NodeList] to list of nodes, since [NodeList] is not iterable. + * + * @return the list of nodes + */ +private fun NodeList.toListOfNodes(): List = (0 until getLength()).map(this::item) + +/** + * Logs the failures for accessibility label check. + * + * @param missingAccessibilityLabelActivities list of Activities missing the accessibility label + * @param accessibilityLabelExemptionTextProtoFilePath the location of the accessibility label + * exemption textproto file. + */ +private fun logFailures( + missingAccessibilityLabelActivities: List, + accessibilityLabelExemptionTextProtoFilePath: String +) { + if (missingAccessibilityLabelActivities.isNotEmpty()) { + println("Accessibility label missing for Activities:") + missingAccessibilityLabelActivities.sorted().forEach { activityPath -> + println("- $activityPath") + } + println() + println( + "If this is correct, please update $accessibilityLabelExemptionTextProtoFilePath.textproto" + ) + println( + "Note that, in general, all Activities should have labels. If you choose to add an" + + " exemption, please specifically call this out in your PR description." + ) + println() + } +} + +/** + * Logs the redundant exemptions. + * + * @param redundantExemptions list of redundant exemptions + * @param accessibilityLabelExemptionTextProtoFilePath the location of the accessibility label + * exemption textproto file. + */ +private fun logRedundantExemptions( + redundantExemptions: List, + accessibilityLabelExemptionTextProtoFilePath: String +) { + if (redundantExemptions.isNotEmpty()) { + println("Redundant exemptions:") + redundantExemptions.sorted().forEach { exemption -> + println("- $exemption") + } + println( + "Please remove them from $accessibilityLabelExemptionTextProtoFilePath.textproto" + ) + println() + } +} + +/** + * Loads the test file exemptions list to proto. + * + * @param pathToProtoBinary path to the pb file to be parsed + * @return proto class from the parsed textproto file + */ +private fun loadAccessibilityLabelExemptionsProto( + pathToProtoBinary: String +): AccessibilityLabelExemptions { + val protoBinaryFile = File(pathToProtoBinary) + val builder = AccessibilityLabelExemptions.getDefaultInstance().newBuilderForType() + + // This cast is type-safe since proto guarantees type consistency from mergeFrom(), + // and this method is bounded by the generic type T. + @Suppress("UNCHECKED_CAST") + val protoObj: AccessibilityLabelExemptions = + FileInputStream(protoBinaryFile).use { + builder.mergeFrom(it) + }.build() as AccessibilityLabelExemptions + return protoObj +} diff --git a/scripts/src/java/org/oppia/android/scripts/label/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/label/BUILD.bazel new file mode 100644 index 00000000000..01524932353 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/label/BUILD.bazel @@ -0,0 +1,17 @@ +""" +Libraries corresponding to accessibility label check that ensures all the activities in the codebase +are defined with labels. +""" + +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library") + +kt_jvm_library( + name = "accessibility_label_check_lib", + testonly = True, + srcs = ["AccessibilityLabelCheck.kt"], + visibility = ["//scripts:oppia_script_binary_visibility"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/common:repository_file", + "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto_lite", + ], +) diff --git a/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel index 8d6093c24be..a7296d63615 100644 --- a/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel @@ -34,15 +34,15 @@ java_lite_proto_library( ) proto_library( - name = "test_file_exemptions_proto", + name = "script_exemptions_proto", srcs = ["script_exemptions.proto"], visibility = ["//scripts:oppia_script_binary_visibility"], ) java_lite_proto_library( - name = "test_file_exemptions_java_proto_lite", + name = "script_exemptions_java_proto_lite", visibility = ["//scripts:oppia_script_library_visibility"], - deps = [":test_file_exemptions_proto"], + deps = [":script_exemptions_proto"], ) proto_library( diff --git a/scripts/src/java/org/oppia/android/scripts/proto/script_exemptions.proto b/scripts/src/java/org/oppia/android/scripts/proto/script_exemptions.proto index 9b0f45a8374..f524bf272b9 100644 --- a/scripts/src/java/org/oppia/android/scripts/proto/script_exemptions.proto +++ b/scripts/src/java/org/oppia/android/scripts/proto/script_exemptions.proto @@ -19,3 +19,19 @@ message TestFileExemptions { // at the correct lexicographical position in the textproto file. repeated string exempted_file_path = 1; } + +// Exemptions for the accessibility label check. Exemptions indicate Activities for which we should +// not check for the presence of an accessibility label. +message AccessibilityLabelExemptions { + // List of all the Activities which should be exempted for the accessibility label check. + // For exempting any Activity, provide its relative path to root in + // 'script/assets/accessibility_label_exemptions.textproto' + // Also, note that the exemptions in the textproto file are maintained in lexicographical order. + // While adding any new Activity, please add it only at the correct lexicographical position, + // so that the list remains sorted. + // + // For example if we want to add the 'RecentlyPlayedActivity' to the exemption list, add: + // 'exempted_activity: "app/src/main/java/org/oppia/android/app/home/HomeActivity"' + // at the correct lexicographical position in the textproto file. + repeated string exempted_activity = 1; +} diff --git a/scripts/src/java/org/oppia/android/scripts/testfile/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/testfile/BUILD.bazel index 57d2018c905..dbcb1023446 100644 --- a/scripts/src/java/org/oppia/android/scripts/testfile/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/testfile/BUILD.bazel @@ -12,6 +12,6 @@ kt_jvm_library( visibility = ["//scripts:oppia_script_binary_visibility"], deps = [ "//scripts/src/java/org/oppia/android/scripts/common:repository_file", - "//scripts/src/java/org/oppia/android/scripts/proto:test_file_exemptions_java_proto_lite", + "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto_lite", ], ) diff --git a/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt new file mode 100644 index 00000000000..f24a27fa786 --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/label/AccessibilityLabelCheckTest.kt @@ -0,0 +1,474 @@ +package org.oppia.android.scripts.label + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.oppia.android.scripts.proto.AccessibilityLabelExemptions +import org.oppia.android.testing.assertThrows +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.PrintStream + +/** Tests for [AccessibilityLabelCheck]. */ +class AccessibilityLabelCheckTest { + private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() + private val originalOut: PrintStream = System.out + private val ACCESSIBILITY_LABEL_CHECK_PASSED_OUTPUT_INDICATOR = "ACCESSIBILITY LABEL CHECK PASSED" + private val ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR = "ACCESSIBILITY LABEL CHECK FAILED" + private val pathToProtoBinary = "scripts/assets/accessibility_label_exemptions.pb" + private val failureNotePartOne = "If this is correct, please update " + + "scripts/assets/accessibility_label_exemptions.textproto" + private val failureNotePartTwo = "Note that, in general, all Activities should have labels. " + + "If you choose to add an exemption, please specifically call this out in your PR description." + + @Rule + @JvmField + var tempFolder = TemporaryFolder() + + @Before + fun setUp() { + tempFolder.newFolder("testfiles") + tempFolder.newFolder("scripts", "assets") + tempFolder.newFile(pathToProtoBinary) + System.setOut(PrintStream(outContent)) + } + + @After + fun restoreStreams() { + System.setOut(originalOut) + } + + @Test + fun testAccessibilityLabel_labelPresent_checkShouldPass() { + val testContent = + """ + + + + + """.trimIndent() + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "splash" + ) + val tempFileRelativePath = "app/src/main/java/org/oppia/android/splash/AndroidManifest.xml" + val manifestFile = tempFolder.newFile("testfiles/$tempFileRelativePath") + manifestFile.writeText(testContent) + + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + tempFileRelativePath + ) + + assertThat(outContent.toString().trim()).isEqualTo( + ACCESSIBILITY_LABEL_CHECK_PASSED_OUTPUT_INDICATOR + ) + } + + @Test + fun testAccessibilityLabel_activityReferenceIsRelative_labelNotPresent_checkShouldFail() { + val testContent = + """ + + + + + """.trimIndent() + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "splash" + ) + val tempFileRelativePath = "app/src/main/java/org/oppia/android/splash/AndroidManifest.xml" + val manifestFile = tempFolder.newFile("testfiles/$tempFileRelativePath") + manifestFile.writeText(testContent) + + val exception = assertThrows(Exception::class) { + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + tempFileRelativePath + ) + } + + assertThat(exception).hasMessageThat().contains( + ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR + ) + val activityRelativePath = "app/src/main/java/org/oppia/android/splash/SecondSplashActivity" + val failureMessage = + """ + Accessibility label missing for Activities: + - $activityRelativePath + + $failureNotePartOne + $failureNotePartTwo + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + @Test + fun testAccessibilityLabel_activityReferenceIsFullyQualified_labelNotPresent_checkShouldFail() { + val testContent = + """ + + + + + """.trimIndent() + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "splash" + ) + val tempFileRelativePath = "app/src/main/java/org/oppia/android/splash/AndroidManifest.xml" + val manifestFile = tempFolder.newFile("testfiles/$tempFileRelativePath") + manifestFile.writeText(testContent) + + val exception = assertThrows(Exception::class) { + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + tempFileRelativePath + ) + } + + assertThat(exception).hasMessageThat().contains( + ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR + ) + val activityRelativePath = "app/src/main/java/org/oppia/android/splash/SecondSplashActivity" + val failureMessage = + """ + Accessibility label missing for Activities: + - $activityRelativePath + + $failureNotePartOne + $failureNotePartTwo + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + @Test + fun testAccessibilityLabel_passMultipleManifests_allLabelsAreDefined_checkShouldPass() { + val testContent1 = + """ + + + + + """.trimIndent() + val testContent2 = + """ + + + + + """.trimIndent() + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "app" + ) + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "splash" + ) + val appManifestPath = "app/src/main/AndroidManifest.xml" + val splashManifestPath = "app/src/main/java/org/oppia/android/splash/AndroidManifest.xml" + val appManifestFile = tempFolder.newFile("testfiles/$appManifestPath") + val splashManifestFile = tempFolder.newFile("testfiles/$splashManifestPath") + appManifestFile.writeText(testContent1) + splashManifestFile.writeText(testContent2) + + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + appManifestPath, + splashManifestPath + ) + + assertThat(outContent.toString().trim()).isEqualTo( + ACCESSIBILITY_LABEL_CHECK_PASSED_OUTPUT_INDICATOR + ) + } + + @Test + fun testAccessibilityLabel_passMultipleManifests_labelsNotDefined_allFailuresShouldBeLogged() { + val testContent1 = + """ + + + + + """.trimIndent() + val testContent2 = + """ + + + + + """.trimIndent() + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "app" + ) + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "splash" + ) + val appManifestPath = "app/src/main/AndroidManifest.xml" + val splashManifestPath = "app/src/main/java/org/oppia/android/splash/AndroidManifest.xml" + val appManifestFile = tempFolder.newFile("testfiles/$appManifestPath") + val splashManifestFile = tempFolder.newFile("testfiles/$splashManifestPath") + appManifestFile.writeText(testContent1) + splashManifestFile.writeText(testContent2) + + val exception = assertThrows(Exception::class) { + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + appManifestPath, + splashManifestPath + ) + } + + assertThat(exception).hasMessageThat().contains( + ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR + ) + val appActivityPath = "app/src/main/java/org/oppia/android/app/TempActivity" + val splashActivityPath = "app/src/main/java/org/oppia/android/splash/SecondSplashActivity" + val failureMessage = + """ + Accessibility label missing for Activities: + - $appActivityPath + - $splashActivityPath + + $failureNotePartOne + $failureNotePartTwo + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + @Test + fun testAccessibilityLabel_multipleFailures_logsShouldBeLexicographicallySorted() { + val testContent1 = + """ + + + + + + + """.trimIndent() + val testContent2 = + """ + + + + + """.trimIndent() + tempFolder.newFolder("testfiles", "app", "src", "main") + tempFolder.newFolder( + "testfiles", "app", "src", "main", "java", "org", "oppia", "android", "splash" + ) + val appManifestPath = "app/src/main/AndroidManifest.xml" + val splashManifestPath = "app/src/main/java/org/oppia/android/splash/AndroidManifest.xml" + val appManifestFile = tempFolder.newFile("testfiles/$appManifestPath") + val splashManifestFile = tempFolder.newFile("testfiles/$splashManifestPath") + appManifestFile.writeText(testContent1) + splashManifestFile.writeText(testContent2) + + val exception = assertThrows(Exception::class) { + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + appManifestPath, + splashManifestPath + ) + } + + assertThat(exception).hasMessageThat().contains( + ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR + ) + val firstAppActivityPath = "app/src/main/java/org/oppia/android/app/FirstTempActivity" + val thirdAppActivityPath = "app/src/main/java/org/oppia/android/app/ThirdTempActivity" + val fourthAppActivityPath = "app/src/main/java/org/oppia/android/app/FourthTempActivity" + val splashActivityPath = "app/src/main/java/org/oppia/android/splash/SecondSplashActivity" + val failureMessage = + """ + Accessibility label missing for Activities: + - $firstAppActivityPath + - $fourthAppActivityPath + - $thirdAppActivityPath + - $splashActivityPath + + $failureNotePartOne + $failureNotePartTwo + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + @Test + fun testAccessibilityLabel_accessibilityLabelNotDefinedForExemptedActivity_checkShouldPass() { + val testContent = + """ + + + + """.trimIndent() + tempFolder.newFolder("testfiles", "app", "src", "main") + val tempFileRelativePath = "app/src/main/AndroidManifest.xml" + val manifestFile = tempFolder.newFile("testfiles/$tempFileRelativePath") + manifestFile.writeText(testContent) + val exemptionFile = File("${tempFolder.root}/$pathToProtoBinary") + val exemptions = AccessibilityLabelExemptions.newBuilder().apply { + this.addAllExemptedActivity( + listOf("app/src/main/java/org/oppia/android/app/home/HomeActivity") + ) + }.build() + exemptions.writeTo(exemptionFile.outputStream()) + + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + tempFileRelativePath + ) + + assertThat(outContent.toString().trim()).isEqualTo( + ACCESSIBILITY_LABEL_CHECK_PASSED_OUTPUT_INDICATOR + ) + } + + @Test + fun testAccessibilityLabel_addRedundantExemption_checkShouldFail() { + val testContent = + """ + + + + """.trimIndent() + tempFolder.newFolder("testfiles", "app", "src", "main") + val tempFileRelativePath = "app/src/main/AndroidManifest.xml" + val manifestFile = tempFolder.newFile("testfiles/$tempFileRelativePath") + manifestFile.writeText(testContent) + val exemptionFile = File("${tempFolder.root}/$pathToProtoBinary") + val exemptions = AccessibilityLabelExemptions.newBuilder().apply { + this.addAllExemptedActivity( + listOf("app/src/main/java/org/oppia/android/app/home/HomeActivity") + ) + }.build() + exemptions.writeTo(exemptionFile.outputStream()) + + val exception = assertThrows(Exception::class) { + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + tempFileRelativePath + ) + } + + assertThat(exception).hasMessageThat().contains( + ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR + ) + val failureMessage = + """ + Redundant exemptions: + - app/src/main/java/org/oppia/android/app/home/HomeActivity + Please remove them from scripts/assets/accessibility_label_exemptions.textproto + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + @Test + fun testAccessibilityLabel_addRedundantExemption_activityMissingLabel_allFailuresShouldLog() { + val testContent = + """ + + + + """.trimIndent() + tempFolder.newFolder("testfiles", "app", "src", "main") + val tempFileRelativePath = "app/src/main/AndroidManifest.xml" + val manifestFile = tempFolder.newFile("testfiles/$tempFileRelativePath") + manifestFile.writeText(testContent) + val activityPath = "app/src/main/java/org/oppia/android/app/home/SplashActivity" + val exemptionFile = File("${tempFolder.root}/$pathToProtoBinary") + val exemptions = AccessibilityLabelExemptions.newBuilder().apply { + this.addAllExemptedActivity( + listOf("app/src/main/java/org/oppia/android/app/home/HomeActivity") + ) + }.build() + exemptions.writeTo(exemptionFile.outputStream()) + + val exception = assertThrows(Exception::class) { + main( + retrieveTestFilesDirectoryPath(), + "${tempFolder.root}/$pathToProtoBinary", + tempFileRelativePath + ) + } + + assertThat(exception).hasMessageThat().contains( + ACCESSIBILITY_LABEL_CHECK_FAILED_OUTPUT_INDICATOR + ) + val failureMessage = + """ + Redundant exemptions: + - app/src/main/java/org/oppia/android/app/home/HomeActivity + Please remove them from scripts/assets/accessibility_label_exemptions.textproto + + Accessibility label missing for Activities: + - $activityPath + + $failureNotePartOne + $failureNotePartTwo + """.trimIndent() + assertThat(outContent.toString().trim()).isEqualTo(failureMessage) + } + + /** Retrieves the absolute path of testfiles directory. */ + private fun retrieveTestFilesDirectoryPath(): String { + return "${tempFolder.root}/testfiles" + } +} diff --git a/scripts/src/javatests/org/oppia/android/scripts/label/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/label/BUILD.bazel new file mode 100644 index 00000000000..f67d999d8b7 --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/label/BUILD.bazel @@ -0,0 +1,17 @@ +""" +Tests corresponding to accessibility label check that ensures all the activities in the codebase +are defined with labels. +""" + +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_test") + +kt_jvm_test( + name = "AccessibilityLabelCheckTest", + srcs = ["AccessibilityLabelCheckTest.kt"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/label:accessibility_label_check_lib", + "//testing:assertion_helpers", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +)