From 3287c9b95b9064004bba380f2b5cb496b3b0b135 Mon Sep 17 00:00:00 2001 From: Emma Lautz Date: Tue, 26 Nov 2019 18:01:10 -0800 Subject: [PATCH] Added infrastructure to safely bump flake8 version Added logic in the Flake8Task to rerun the execution with ignoring a set of rules if the first execution fails. It will rerun if the IGNORE_RULES set it non-empty. This set of rules will contain any rules/checks added between flake8 versions. For example, when we bump to flake8 3.8.0, we should update the IGNORE_RULES value to hold all rules and checks added between flake8 3.6.0 and 3.8.0. If the second run, when ignoring newly added rules, succeeds, we now emit a warning to the user stating which rules are being ignored, and which of these ignored rules failed for them. Added a method called overrideIgnoreRules() which is used for integration testings and will be used internally by LI. Integration tests were added to verify this new functionality. --- .../tasks/Flake8TaskIntegrationTest.groovy | 73 +++++++++++++++---- .../AbstractPythonMainSourceDefaultTask.java | 8 +- .../gradle/python/tasks/Flake8Task.java | 40 +++++++++- 3 files changed, 105 insertions(+), 16 deletions(-) diff --git a/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/tasks/Flake8TaskIntegrationTest.groovy b/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/tasks/Flake8TaskIntegrationTest.groovy index 4bed4b1b..5e8e9a51 100644 --- a/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/tasks/Flake8TaskIntegrationTest.groovy +++ b/pygradle-plugin/src/integTest/groovy/com/linkedin/gradle/python/tasks/Flake8TaskIntegrationTest.groovy @@ -26,9 +26,9 @@ 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' @@ -36,7 +36,10 @@ class Flake8TaskIntegrationTest extends Specification { | |${ 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 + } } diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonMainSourceDefaultTask.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonMainSourceDefaultTask.java index 40ebff1b..8ce212ec 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/AbstractPythonMainSourceDefaultTask.java +++ b/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 subArguments = new ArrayList<>(); private PythonExtension extension; private PythonDetails pythonDetails; - private String output; + protected String output; @Input public List additionalArguments = new ArrayList<>(); @@ -135,7 +135,11 @@ public void subArgs(Collection args) { } @TaskAction - public void executePythonProcess() { + public void executeTask() { + executePythonProcess(); + } + + void executePythonProcess() { preExecution(); final TeeOutputContainer container = new TeeOutputContainer(stdOut, errOut); diff --git a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java b/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java index 9e70b52e..ed55ecb6 100644 --- a/pygradle-plugin/src/main/groovy/com/linkedin/gradle/python/tasks/Flake8Task.java +++ b/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 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 ignoreRules) { + this.ignoreRules = ignoreRules; + } + + public Set 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(); + } } }