diff options
2 files changed, 189 insertions, 10 deletions
diff --git a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DemotingTestWithoutBugDetector.kt b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DemotingTestWithoutBugDetector.kt index f64ea4561906..459a38e9318c 100644 --- a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DemotingTestWithoutBugDetector.kt +++ b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DemotingTestWithoutBugDetector.kt @@ -25,6 +25,7 @@ import com.android.tools.lint.detector.api.JavaContext import com.android.tools.lint.detector.api.Scope import com.android.tools.lint.detector.api.Severity import com.android.tools.lint.detector.api.SourceCodeScanner +import java.util.regex.Pattern import org.jetbrains.uast.UAnnotation import org.jetbrains.uast.UElement @@ -36,22 +37,38 @@ class DemotingTestWithoutBugDetector : Detector(), SourceCodeScanner { override fun createUastHandler(context: JavaContext): UElementHandler { return object : UElementHandler() { override fun visitAnnotation(node: UAnnotation) { - if (node.qualifiedName !in DEMOTING_ANNOTATION) { - return + // Annotations having int bugId field + if (node.qualifiedName in DEMOTING_ANNOTATION_BUG_ID) { + val bugId = node.findAttributeValue("bugId")!!.evaluate() as Int + if (bugId <= 0) { + val location = context.getLocation(node) + val message = "Please attach a bug id to track demoted test" + context.report(ISSUE, node, location, message) + } } - val bugId = node.findAttributeValue("bugId")!!.evaluate() as Int - if (bugId <= 0) { - val location = context.getLocation(node) - val message = "Please attach a bug id to track demoted test" - context.report(ISSUE, node, location, message) + // @Ignore has a String field for reason + if (node.qualifiedName == DEMOTING_ANNOTATION_IGNORE) { + val reason = node.findAttributeValue("value")!!.evaluate() as String + val bugPattern = Pattern.compile("b/\\d+") + if (!bugPattern.matcher(reason).find()) { + val location = context.getLocation(node) + val message = "Please attach a bug (e.g. b/123) to track demoted test" + context.report(ISSUE, node, location, message) + } } } } } companion object { - val DEMOTING_ANNOTATION = - listOf("androidx.test.filters.FlakyTest", "android.platform.test.annotations.FlakyTest") + val DEMOTING_ANNOTATION_BUG_ID = + listOf( + "androidx.test.filters.FlakyTest", + "android.platform.test.annotations.FlakyTest", + "android.platform.test.rule.PlatinumRule.Platinum", + ) + + const val DEMOTING_ANNOTATION_IGNORE = "org.junit.Ignore" @JvmField val ISSUE: Issue = diff --git a/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DemotingTestWithoutBugDetectorTest.kt b/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DemotingTestWithoutBugDetectorTest.kt index 557c300635eb..63eb2632979c 100644 --- a/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DemotingTestWithoutBugDetectorTest.kt +++ b/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DemotingTestWithoutBugDetectorTest.kt @@ -127,6 +127,139 @@ class DemotingTestWithoutBugDetectorTest : SystemUILintDetectorTest() { ) } + @Test + fun testExcludeDevices_withBugId() { + lint() + .files( + TestFiles.java( + """ + package test.pkg; + import android.platform.test.rule.PlatinumRule.Platinum; + + @Platinum(devices = "foo,bar", bugId = 123) + public class TestClass { + public void testCase() {} + } + """ + ) + .indented(), + *stubs + ) + .issues(DemotingTestWithoutBugDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun testExcludeDevices_withoutBugId() { + lint() + .files( + TestFiles.java( + """ + package test.pkg; + import android.platform.test.rule.PlatinumRule.Platinum; + + @Platinum(devices = "foo,bar") + public class TestClass { + public void testCase() {} + } + """ + ) + .indented(), + *stubs + ) + .issues(DemotingTestWithoutBugDetector.ISSUE) + .run() + .expect( + """ + src/test/pkg/TestClass.java:4: Warning: Please attach a bug id to track demoted test [DemotingTestWithoutBug] + @Platinum(devices = "foo,bar") + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + ) + } + + @Test + fun testIgnore_withBug() { + lint() + .files( + TestFiles.java( + """ + package test.pkg; + import org.junit.Ignore; + + @Ignore("Blocked by b/123.") + public class TestClass { + public void testCase() {} + } + """ + ) + .indented(), + *stubs + ) + .issues(DemotingTestWithoutBugDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun testIgnore_withoutBug() { + lint() + .files( + TestFiles.java( + """ + package test.pkg; + import org.junit.Ignore; + + @Ignore + public class TestClass { + public void testCase() {} + } + """ + ) + .indented(), + *stubs + ) + .issues(DemotingTestWithoutBugDetector.ISSUE) + .run() + .expect( + """ + src/test/pkg/TestClass.java:4: Warning: Please attach a bug (e.g. b/123) to track demoted test [DemotingTestWithoutBug] + @Ignore + ~~~~~~~ + 0 errors, 1 warnings + """ + ) + + lint() + .files( + TestFiles.java( + """ + package test.pkg; + import org.junit.Ignore; + + @Ignore("Not ready") + public class TestClass { + public void testCase() {} + } + """ + ) + .indented(), + *stubs + ) + .issues(DemotingTestWithoutBugDetector.ISSUE) + .run() + .expect( + """ + src/test/pkg/TestClass.java:4: Warning: Please attach a bug (e.g. b/123) to track demoted test [DemotingTestWithoutBug] + @Ignore("Not ready") + ~~~~~~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + ) + } + private val filtersFlakyTestStub: TestFile = java( """ @@ -147,5 +280,34 @@ class DemotingTestWithoutBugDetectorTest : SystemUILintDetectorTest() { } """ ) - private val stubs = arrayOf(filtersFlakyTestStub, annotationsFlakyTestStub) + private val annotationsPlatinumStub: TestFile = + java( + """ + package android.platform.test.rule; + + public class PlatinumRule { + public @interface Platinum { + String devices(); + int bugId() default -1; + } + } + """ + ) + private val annotationsIgnoreStub: TestFile = + java( + """ + package org.junit; + + public @interface Ignore { + String value() default ""; + } + """ + ) + private val stubs = + arrayOf( + filtersFlakyTestStub, + annotationsFlakyTestStub, + annotationsPlatinumStub, + annotationsIgnoreStub + ) } |