Skip to content

Commit

Permalink
Fix #3498: Add CI check to ensure license text is never checked into …
Browse files Browse the repository at this point in the history
…Git (#3632)

* Call out dependency names when license details incomplete exception is thrown

* Modify message to include rerunning the script after updating maven_dependencies.textproto

* Correct typo

* Add suggested changes

* Add full stop

* Correct comment sentence in versions.bzl

* Add some more test cases and add suggested changes

* Separate all methods in a new file

* Correct imports

* Update to reflect changes of coordinate name

* Add CI check

* Correct static_checks.yml

* Remove empty line

* Correct visibility of bazel targets and delete one argument from the script

* Correct path in exemptions file

* Add import for LicenseFetcher in test file

* Add suggested changes

* Add suggested change

* correct kdoc

* Use associateWith instead of associateTo

* Resolve conflicts

* Use associateWith instead of associateTo

* Add suggested changes

* Add tests for MavenDependenciesListCheck

* Remove unwanted lines

* Add test file for MavenDependenciesListGenerator

* Add test cases for MavenDependenciesListGenerator.kt

* Correct dep name

* Fix lint

* Add KDocs and more test cases

* Revert changes in maven_dependencies.textproto

* Correct name of binary in static_checks.yml

* Correct name of Checks

* Run GenerateMavenDependenciesList.kt to update maven_dependencies.textproto

* Modify message to include wiki link and to run the GenerateMavenDependenciesList.kt script

* Add more test cases

* Add more test cases

* Add CI check

* Add suggested changes

* Fix newly added test case

* Refactor check into a new package

* Refactor check to new package

* Correct script library

* Fix deps and main_class attribute in root BUILD

* Add @BenHenning as the CODEOWNER for third_party_dependencies.xml

* Add suggested changes

* Remove extra blank line

* Add suggested changes

* Correct library target in maven_dependencies_check_lib

* Correct paths in test exemptions

* Add suggested changes

* Add suggested changes

* Correct name of the test file

* Add script to run in the CI to also verify license details.

* Correct path

* Remove redundant test case

* Correct import in GenerateMavenDependenciesListTest.kt

* Reorder CI checks

* Reorder CI checks

* Remove unused imports

* Correct package name from data to model

* Correct import in MavenDependenciesRetrieverTest.kt

* Correct import in MavenDependenciesRetrieverTest.kt

* Correct format in static_checks.yml

* Correct name of script in XML comment

* Add suggested changes

* Add check and test cases for catching incomplete license details and remove extra CI check.

* Remove unused code

* Remove omit function from test file

* Correct warning comment

* Add suggested changes

* Add suggested changes
  • Loading branch information
prayutsu authored Aug 14, 2021
1 parent a43e743 commit 7aa1750
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 0 deletions.
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,5 @@ WORKSPACE @BenHenning
.bazelrc @BenHenning
/tools/android/ @BenHenning

# License texts.
/app/src/main/res/values/third_party_dependencies.xml @BenHenning
5 changes: 5 additions & 0 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,8 @@ jobs:
if: always()
run: |
bazel run //scripts:maven_dependencies_list_check -- $(pwd) third_party/maven_install.json scripts/assets/maven_dependencies.pb
- name: License Texts Check
if: always()
run: |
bazel run //scripts:license_texts_check -- $(pwd)/app/src/main/res/values/third_party_dependencies.xml
1 change: 1 addition & 0 deletions app/src/main/res/values/third_party_dependencies.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Do not edit this file. It is generated by running RetrieveLicenseTexts.kt. -->
<resources>
<string name="third_party_dependency_name_0">artifact.name:dependency0</string>
<string name="third_party_dependency_name_1">artifact.name:dependency1</string>
Expand Down
7 changes: 7 additions & 0 deletions scripts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ kt_jvm_binary(
runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/maven:retrieve_license_texts_lib"],
)

kt_jvm_binary(
name = "license_texts_check",
testonly = True,
main_class = "org.oppia.android.scripts.license.LicenseTextsCheckKt",
runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/license:license_texts_check_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"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,10 @@ kt_jvm_library(
"//scripts/src/java/org/oppia/android/scripts/license:maven_dependencies_retriever",
],
)

kt_jvm_library(
name = "license_texts_check_lib",
testonly = True,
srcs = ["LicenseTextsCheck.kt"],
visibility = ["//scripts:oppia_script_binary_visibility"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.oppia.android.scripts.license

import java.io.File

private const val WARNING_COMMENT =
"<!-- Do not edit this file. It is generated by running RetrieveLicenseTexts.kt. -->"

/**
* Script to verify that the actual license texts are never checked into our VCS. It mainly
* verifies that the script-generated version of third_party_dependencies.xml file is never
* checked in.
*
* Usage:
* bazel run //scripts:maven_dependencies_list_check -- <path_to_third_party_deps_xml>
*
* Arguments:
* - path_to_third_party_deps_xml: path to the third_party_dependencies.xml
*
* Example:
* bazel run //scripts:maven_dependencies_list_check -- $(pwd)/app/src/main/res/values/third_party_dependencies.xml
*/
fun main(args: Array<String>) {
if (args.size < 1) {
throw Exception("Too few arguments passed")
}
val pathToThirdPartyDepsXml = args[0]
val thirdPartyDepsXml = File(pathToThirdPartyDepsXml)
check(thirdPartyDepsXml.exists()) { "File does not exist: $thirdPartyDepsXml" }

val xmlContent = thirdPartyDepsXml.readText()

checkIfCommentIsPresent(xmlContent = xmlContent, comment = WARNING_COMMENT)

println("License texts Check Passed")
}

private fun checkIfCommentIsPresent(xmlContent: String, comment: String) {
if (comment !in xmlContent) {
println("Please revert the changes in third_party_dependencies.xml")
throw Exception("License texts potentially checked into VCS")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ Tests corresponding to the maven dependencies and their license checks.

load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_test")

kt_jvm_test(
name = "LicenseTextsCheckTest",
srcs = ["LicenseTextsCheckTest.kt"],
deps = [
"//scripts/src/java/org/oppia/android/scripts/license:license_texts_check_lib",
"//testing:assertion_helpers",
"//third_party:com_google_truth_truth",
"//third_party:org_jetbrains_kotlin_kotlin-test-junit",
],
)

kt_jvm_test(
name = "MavenDependenciesListCheckTest",
size = "large",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package org.oppia.android.scripts.license

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.testing.assertThrows
import java.io.ByteArrayOutputStream
import java.io.PrintStream

/** Tests for [LicenseTextsCheck]. */
class LicenseTextsCheckTest {
private val WARNING_COMMENT =
"<!-- Do not edit this file. It is generated by running RetrieveLicenseTexts.kt. -->"
private val SCRIPT_PASSED_MESSAGE = "License texts Check Passed"
private val LICENSE_TEXTS_CHECKED_IN_FAILURE = "License texts potentially checked into VCS"
private val TOO_FEW_ARGS_PASSED_FAILURE = "Too few arguments passed"

private val outContent: ByteArrayOutputStream = ByteArrayOutputStream()
private val originalOut: PrintStream = System.out

@Rule
@JvmField
var tempFolder = TemporaryFolder()

@Before
fun setUp() {
tempFolder.newFolder("values")
System.setOut(PrintStream(outContent))
}

@After
fun restoreStreams() {
System.setOut(originalOut)
}

@Test
fun testLicenseTexsCheck_noArgsPassed_failWithException() {
val thirdPartyDepsXmlFile = tempFolder.newFile("values/third_party_dependencies.xml")
thirdPartyDepsXmlFile.writeText(WARNING_COMMENT)

val exception = assertThrows(Exception::class) {
main(arrayOf())
}

assertThat(exception).hasMessageThat().contains(TOO_FEW_ARGS_PASSED_FAILURE)
}

@Test
fun testLicenseTexsCheck_emptyXmlFile_checkFailsWithException() {
val thirdPartyDepsXmlFile = tempFolder.newFile("values/third_party_dependencies.xml")
thirdPartyDepsXmlFile.writeText("")

val exception = assertThrows(Exception::class) {
main(arrayOf("${tempFolder.root}/values/third_party_dependencies.xml"))
}

assertThat(exception).hasMessageThat().contains(LICENSE_TEXTS_CHECKED_IN_FAILURE)
}

@Test
fun testLicenseTexsCheck_warningCommentNotPresent_checkFailsWithException() {
val thirdPartyDepsXmlFile = tempFolder.newFile("values/third_party_dependencies.xml")
thirdPartyDepsXmlFile.writeText(
"""
<?xml version="1.0" encoding="utf-8"?>
<resources>
<string name="third_party_dependency_name_0">Glide</string>
</resources>
""".trimIndent()
)

val exception = assertThrows(Exception::class) {
main(arrayOf("${tempFolder.root}/values/third_party_dependencies.xml"))
}

assertThat(exception).hasMessageThat().contains(LICENSE_TEXTS_CHECKED_IN_FAILURE)
}

@Test
fun testLicenseTexsCheck_onlyWarningCommentPresent_checkPasses() {
val thirdPartyDepsXmlFile = tempFolder.newFile("values/third_party_dependencies.xml")
thirdPartyDepsXmlFile.writeText(WARNING_COMMENT)

main(arrayOf("${tempFolder.root}/values/third_party_dependencies.xml"))

assertThat(outContent.toString()).contains(SCRIPT_PASSED_MESSAGE)
}

@Test
fun testLicenseTexsCheck_warningCommentPresentWithSomeXmlCode_checkPasses() {
val thirdPartyDepsXmlFile = tempFolder.newFile("values/third_party_dependencies.xml")
thirdPartyDepsXmlFile.writeText(
"""
<?xml version="1.0" encoding="utf-8"?>
$WARNING_COMMENT
<resources>
<string name="third_party_dependency_name_0">Glide</string>
</resources>
""".trimIndent()
)

main(arrayOf("${tempFolder.root}/values/third_party_dependencies.xml"))

assertThat(outContent.toString()).contains(SCRIPT_PASSED_MESSAGE)
}

@Test
fun testLicenseTexsCheck_xmlFileNotPresent_checkFailsWithFileNotFoundException() {
val pathToThirdPartyDepsXml = "${tempFolder.root}/values/third_party_dependencies.xml"
val exception = assertThrows(Exception::class) {
main(arrayOf("${tempFolder.root}/values/third_party_dependencies.xml"))
}

assertThat(exception).hasMessageThat().contains(
"File does not exist: $pathToThirdPartyDepsXml"
)
}
}

0 comments on commit 7aa1750

Please sign in to comment.