Skip to content

Commit

Permalink
improve error messages (#73)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewparmet authored May 12, 2022
1 parent 360c2e9 commit 2e5498d
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 53 deletions.
7 changes: 6 additions & 1 deletion src/main/kotlin/com/parmet/buf/gradle/Breaking.kt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ private fun Project.configureBreakingTask() {
bufBuildPublicationFile,
"--against",
singleFileFromConfiguration(BUF_BREAKING_CONFIGURATION_NAME)
)
) {
"""
|Some Protobuf files had breaking changes:
|$it
""".trimMargin()
}
}
}
25 changes: 22 additions & 3 deletions src/main/kotlin/com/parmet/buf/gradle/BufSupport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.kotlin.dsl.ivy
import org.gradle.kotlin.dsl.repositories
import java.io.ByteArrayOutputStream
import java.nio.charset.StandardCharsets

const val BUF_BINARY_CONFIGURATION_NAME = "bufTool"

Expand Down Expand Up @@ -54,17 +56,19 @@ internal fun Project.configureBufDependency() {
)
}

internal fun Task.execBuf(vararg args: Any) {
execBuf(args.asList())
internal fun Task.execBuf(vararg args: Any, customErrorMessage: ((String) -> String)? = null) {
execBuf(args.asList(), customErrorMessage)
}

internal fun Task.execBuf(args: Iterable<Any>) {
internal fun Task.execBuf(args: Iterable<Any>, customErrorMessage: ((String) -> String)? = null) {
if (project.hasProtobufGradlePlugin()) {
dependsOn(CREATE_SYM_LINKS_TO_MODULES_TASK_NAME)
dependsOn(WRITE_WORKSPACE_YAML_TASK_NAME)
}
doLast {
with(project) {
val out = ByteArrayOutputStream()

exec {
val executable = singleFileFromConfiguration(BUF_BINARY_CONFIGURATION_NAME)

Expand All @@ -82,8 +86,23 @@ internal fun Task.execBuf(args: Iterable<Any>) {

commandLine(executable)
setArgs(args)
isIgnoreExitValue = true

if (customErrorMessage != null) {
standardOutput = out
}

logger.info("Running buf from $workingDir: `buf ${args.joinToString(" ")}`")
}.also {
if (customErrorMessage == null) {
it.assertNormalExitValue()
} else {
if (it.exitValue != 0) {
throw IllegalStateException(
customErrorMessage(out.toByteArray().toString(StandardCharsets.UTF_8))
)
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,34 @@ import org.gradle.api.Task
import java.nio.file.Path

internal fun Task.configureDirectorySpecificBufExecution(
vararg bufCommand: String
vararg bufCommand: String,
customErrorMessage: ((String) -> String)? = null
) {
configureDirectorySpecificBufExecution(bufCommand.asList(), emptyList())
configureDirectorySpecificBufExecution(bufCommand.asList(), emptyList(), customErrorMessage)
}

internal fun Task.configureDirectorySpecificBufExecution(
bufCommand: String,
extraArgs: Iterable<String>
extraArgs: Iterable<String>,
customErrorMessage: ((String) -> String)
) {
configureDirectorySpecificBufExecution(listOf(bufCommand), extraArgs)
configureDirectorySpecificBufExecution(listOf(bufCommand), extraArgs, customErrorMessage)
}

private fun Task.configureDirectorySpecificBufExecution(
bufCommand: Iterable<String>,
extraArgs: Iterable<String>
extraArgs: Iterable<String>,
customErrorMessage: ((String) -> String)? = null
) {
fun lintWithArgs(path: Path? = null) =
fun runWithArgs(path: Path? = null) =
bufCommand + listOfNotNull(path?.let(::mangle)) + extraArgs

when {
project.hasProtobufGradlePlugin() ->
project.srcProtoDirs().forEach { execBuf(lintWithArgs(it)) }
project.srcProtoDirs().forEach { execBuf(runWithArgs(it), customErrorMessage) }
project.hasWorkspace() ->
execBuf(bufCommand)
execBuf(bufCommand, customErrorMessage)
else ->
execBuf(lintWithArgs())
execBuf(runWithArgs(), customErrorMessage)
}
}
8 changes: 7 additions & 1 deletion src/main/kotlin/com/parmet/buf/gradle/Format.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ private fun Project.configureBufFormatCheck() {
description = "Checks that a Protobuf schema is formatted according to Buf's formatting rules."
enabled = getExtension().enforceFormat

configureDirectorySpecificBufExecution("format", "-d", "--exit-code")
configureDirectorySpecificBufExecution("format", "-d", "--exit-code") {
"""
|Some Protobuf files had format violations:
|$it
|Run './gradlew :bufFormatApply' to fix these violations.
""".trimMargin()
}
}

tasks.named(CHECK_TASK_NAME).dependsOn(BUF_FORMAT_CHECK_TASK_NAME)
Expand Down
7 changes: 6 additions & 1 deletion src/main/kotlin/com/parmet/buf/gradle/Lint.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ internal fun Project.configureLint() {
configureDirectorySpecificBufExecution(
"lint",
bufConfigFile()?.let { listOf("--config", it.readText()) }.orEmpty()
)
) {
"""
|Some Protobuf files had lint violations:
|$it
""".trimMargin()
}
}

tasks.named(CHECK_TASK_NAME).dependsOn(BUF_LINT_TASK_NAME)
Expand Down
6 changes: 6 additions & 0 deletions src/test/kotlin/com/parmet/buf/gradle/AbstractBreakingTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ abstract class AbstractBreakingTest : AbstractBufIntegrationTest() {

val result = checkRunner().buildAndFail()
assertThat(result.task(":$BUF_BREAKING_TASK_NAME")?.outcome).isEqualTo(FAILED)
assertThat(result.output).contains(
"""
Execution failed for task ':bufBreaking'.
> Some Protobuf files had breaking changes:
""".trimIndent()
)
assertThat(result.output).contains("Previously present message \"BasicMessage\" was deleted from file.")
}

Expand Down
29 changes: 14 additions & 15 deletions src/test/kotlin/com/parmet/buf/gradle/AbstractFormatCheckTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,33 +29,32 @@ abstract class AbstractFormatCheckTest : AbstractBufIntegrationTest() {

@Test
fun `format an incorrect message`() {
baseAssertBadWhitespace()
assertBadWhitespace()
}

@Test
fun `do not format an incorrect message when enforcement is disabled`() {
baseAssertBadWhitespace()
assertBadWhitespace()

buildFile.replace("enforceFormat = true", "enforceFormat = false")

assertSuccess()
}

private fun baseAssertBadWhitespace() {
assertBadWhitespace(
"""
-message Foo {
-
-}
+message Foo {}
""".trimIndent()
)
}

protected fun assertBadWhitespace(diff: String) {
protected fun assertBadWhitespace() {
val result = checkRunner().buildAndFail()
assertThat(result.task(":$BUF_FORMAT_CHECK_TASK_NAME")?.outcome).isEqualTo(FAILED)
assertThat(result.output).contains(diff)
assertThat(result.output)
.contains(
"""
| -message Foo {
| -
| -}
| +message Foo {}
|
| Run './gradlew :bufFormatApply' to fix these violations.
""".trimMargin()
)
}

protected fun assertSuccess() {
Expand Down
6 changes: 6 additions & 0 deletions src/test/kotlin/com/parmet/buf/gradle/AbstractLintTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ abstract class AbstractLintTest : LintTestUtilities, AbstractBufIntegrationTest(
private fun assertBadEnumSuffix() {
val result = checkRunner().buildAndFail()
assertThat(result.task(":$BUF_LINT_TASK_NAME")?.outcome).isEqualTo(FAILED)
assertThat(result.output).contains(
"""
Execution failed for task ':bufLint'.
> Some Protobuf files had lint violations:
""".trimIndent()
)
assertThat(result.output).contains("Enum zero value name \"TEST_FOO\" should be suffixed with \"_UNSPECIFIED\"")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,7 @@ class FormatCheckWithProtobufGradleTest : AbstractFormatCheckTest() {

@Test
fun `format a bad separate protobuf source directory through the protobuf-gradle-plugin`() {
assertBadWhitespace(
"""
-message BasicMessage {
-
-
-}
+message BasicMessage {}
""".trimIndent()
)
assertBadWhitespace()
}

@Test
Expand All @@ -53,15 +45,6 @@ class FormatCheckWithProtobufGradleTest : AbstractFormatCheckTest() {

@Test
fun `format a bad file with a protobuf dependency and a google dependency with the protobuf-gradle-plugin`() {
assertBadWhitespace(
"""
message BasicMessage {
-
-
google.protobuf.Any any = 1;
protokt.ProtoktFileOptions protokt_file_options = 2;
}
""".trimIndent()
)
assertBadWhitespace()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ import "google/protobuf/any.proto";
import "protokt/protokt.proto";

message BasicMessage {


google.protobuf.Any any = 1;
protokt.ProtoktFileOptions protokt_file_options = 2;
}

message Foo {

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ syntax = "proto3";

package parmet.buf.test.v1;

message BasicMessage {

message Foo {

}

0 comments on commit 2e5498d

Please sign in to comment.