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(); + } } }