-
Notifications
You must be signed in to change notification settings - Fork 528
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
* 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 b72e419 * 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
- Loading branch information
1 parent
04bc03f
commit 6610306
Showing
11 changed files
with
807 additions
and
6 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
203 changes: 203 additions & 0 deletions
203
scripts/src/java/org/oppia/android/scripts/label/AccessibilityLabelCheck.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 -- <path_to_directory_root> | ||
| * <path_to_proto_binary> [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<Node> = (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<String>, | ||
| 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<String>, | ||
| 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 | ||
| } |
17 changes: 17 additions & 0 deletions
17
scripts/src/java/org/oppia/android/scripts/label/BUILD.bazel
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", | ||
| ], | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.