Merge pull request #333 from elautz/flake8-infra

Added infrastructure to safely bump flake8 version

pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/tasks/Flake8TaskIntegrationTest.groovy

@@ -26,17 +26,20 @@ class Flake8TaskIntegrationTest extends Specification {
 
     @Rule
     final DefaultProjectLayoutRule testProjectDir = new DefaultProjectLayoutRule()
+    def bazPy
 
-    def "a passing flake8"() {
-        given:
+    def setup() {
         testProjectDir.buildFile << """\
         |plugins {
         |    id 'com.linkedin.python'
         |}
         |
         |${ PyGradleTestBuilder.createRepoClosure() }
         """.stripMargin().stripIndent()
+        bazPy = new File(testProjectDir.root, 'foo/src/foo/baz.py')
+    }
 
+    def "a passing flake8"() {
         when:
         def result = GradleRunner.create()
             .withProjectDir(testProjectDir.root)
@@ -46,22 +49,12 @@ class Flake8TaskIntegrationTest extends Specification {
         println result.output
 
         then:
-
         result.task(':foo:flake8').outcome == TaskOutcome.SUCCESS
     }
 
     def "a failing flake8"() {
         given:
-        testProjectDir.buildFile << """\
-        |plugins {
-        |    id 'com.linkedin.python'
-        |}
-        |
-        |${ PyGradleTestBuilder.createRepoClosure() }
-        """.stripMargin().stripIndent()
-
-        def baxPy = new File(testProjectDir.root, 'foo/src/foo/baz.py')
-        baxPy.text = '''
+        bazPy.text = '''
         |import os, sys
         '''.stripMargin().stripIndent()
 
@@ -77,4 +70,58 @@ class Flake8TaskIntegrationTest extends Specification {
         result.output.contains('baz.py:2:10: E401 multiple imports on one line')
         result.task(':foo:flake8').outcome == TaskOutcome.FAILED
     }
+
+    def "flake8 fails even with ignore"() {
+        given:
+        testProjectDir.buildFile << '''
+        | import com.linkedin.gradle.python.tasks.Flake8Task
+        | tasks.withType(Flake8Task) { Flake8Task task ->
+        |     task.setIgnoreRules(["E401"] as Set)
+        | }
+        '''.stripMargin().stripIndent()
+
+        bazPy.text = '''
+        |import os, sys
+        |'''.stripMargin().stripIndent()
+
+        when:
+        def result = GradleRunner.create()
+            .withProjectDir(testProjectDir.root)
+            .withArguments('flake8', '-s', '-i')
+            .withPluginClasspath()
+            .buildAndFail()
+        println result.output
+
+        then:
+        result.output.contains("baz.py:2:1: F401 'os' imported but unused")
+        result.task(':foo:flake8').outcome == TaskOutcome.FAILED
+    }
+
+    def "warning for a newly failing flake8"() {
+        given:
+        testProjectDir.buildFile << '''
+        | import com.linkedin.gradle.python.tasks.Flake8Task
+        | tasks.withType(Flake8Task) { Flake8Task task ->
+        |     task.setIgnoreRules(["E401", "F401"] as Set)
+        | }
+        '''.stripMargin().stripIndent()
+
+        bazPy.text = '''
+        |import os, sys
+        |'''.stripMargin().stripIndent()
+
+        when:
+        def result = GradleRunner.create()
+            .withProjectDir(testProjectDir.root)
+            .withArguments('flake8', '-s', '-i')
+            .withPluginClasspath()
+            .build()
+        println result.output
+
+        then:
+        result.output.contains('The flake8 version has been recently updated, which added the following new rules:')
+        result.output.contains('baz.py:2:10: E401 multiple imports on one line')
+        result.output.contains("baz.py:2:1: F401 'os' imported but unused")
+        result.task(':foo:flake8').outcome == TaskOutcome.SUCCESS
+    }
 }

pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonMainSourceDefaultTask.java

@@ -55,7 +55,7 @@ abstract public class AbstractPythonMainSourceDefaultTask extends DefaultTask im
     private List<String> subArguments = new ArrayList<>();
     private PythonExtension extension;
     private PythonDetails pythonDetails;
-    private String output;
+    protected String output;
 
     @Input
     public List<String> additionalArguments = new ArrayList<>();
@@ -135,7 +135,11 @@ public void subArgs(Collection<String> args) {
     }
 
     @TaskAction
-    public void executePythonProcess() {
+    public void executeTask() {
+        executePythonProcess();
+    }
+
+    void executePythonProcess() {
         preExecution();
 
         final TeeOutputContainer container = new TeeOutputContainer(stdOut, errOut);

pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java

@@ -17,6 +17,8 @@
 
 import com.linkedin.gradle.python.PythonExtension;
 import com.linkedin.gradle.python.util.ExtensionUtils;
+import java.util.HashSet;
+import java.util.Set;
 import org.apache.commons.io.FileUtils;
 import org.gradle.api.logging.Logger;
 import org.gradle.api.logging.Logging;
@@ -32,8 +34,31 @@
 public class Flake8Task extends AbstractPythonMainSourceDefaultTask {
 
     private static final Logger log = Logging.getLogger(Flake8Task.class);
+    // Track whether the current run is excluding the new rules
+    private Boolean ignoringNewRules = false;
+    private String firstRunOutput = null;
+
+    // Set of flake8 rules to ignore (i.e. warn if these checks fail, rather than failing the task)
+    private Set<String> ignoreRules = new HashSet<>();
+
+    private static final String IGNORED_RULES_MSG = "######################### WARNING ##########################\n"
+        + "The flake8 version has been recently updated, which added the following new rules:\n"
+        + "%s\n"  // This will be replaced with the set of ignored rules
+        + "Your project is failing for one or more of these rules. Please address them, as they will be enforced soon.\n"
+        + "%s############################################################\n";
+
+    // Provide the ability to set the ignored rules for testing purposes,
+    // but mainly so that users can enforce the use of a different version of flake8 in their plugins.
+    public void setIgnoreRules(Set<String> ignoreRules) {
+        this.ignoreRules = ignoreRules;
+    }
+
+    public Set<String> getIgnoreRules() {
+        return ignoreRules;
+    }
 
     public void preExecution() {
+        ignoreExitValue = true;
         PythonExtension pythonExtension = ExtensionUtils.getPythonExtension(getProject());
         File flake8Exec = pythonExtension.getDetails().getVirtualEnvironment().findExecutable("flake8");
 
@@ -77,6 +102,19 @@ public void preExecution() {
 
     @Override
     public void processResults(ExecResult execResult) {
-        //Not needed
+        // If the first run of flake8 fails, trying running it again but ignoring the
+        // rules/checks added by the previous version bump.
+        if ((execResult.getExitValue() != 0) && !ignoringNewRules && (ignoreRules.size() > 0)) {
+            ignoringNewRules = true;
+            firstRunOutput = this.output;
+            subArgs("--extend-ignore=" + String.join(",", ignoreRules));
+            executePythonProcess();
+        } else if ((execResult.getExitValue() == 0) && ignoringNewRules) {
+            // The previous run failed, but flake8 succeeds when we ignore the most recent rules.
+            // Warn the user that they are failing one or more of the new rules.
+            log.warn(String.format(IGNORED_RULES_MSG, String.join(", ", ignoreRules), firstRunOutput));
+        } else {
+            execResult.assertNormalExitValue();
+        }
     }
 }