CodeGenerator unit test

[nbaztec]

A follow up from #104 on #48

The approach is as follows:

• Obtain FileDescriptorSet from proto compilation
  • Use protoc provided in PATH, if not present download it
  • Use resources/*.proto files to generate temporary descriptor_set
• Parse the List<FileDescriptorProto> for CodeGenerator
• Use KotlinCompilation to verify generated code

Open to suggestions.

[nbaztec]

@garyp any opinions?

[garyp]

This is great @nbaztec. Thanks! This will make it much easier to add unit tests of the code generator.

My only feedback is that it'd be better to do the protoc download and protoc execution as gradle tasks rather than as part of the unit test. That will allow the results to be cached properly by gradle rather than having to be rerun every time the tests run. Specifically:

• Create a gradle task that uses ProtocTask to run protoc and generate the descriptor set into a file under the gradle build directory. The tests can then depend on this gradle task.

• The current ProtocTask doesn't download protoc if it doesn't already exist. That has been a bit of an annoyance and it'd be nice if it downloaded protoc, though you don't have to implement it in this PR if you don't want to. If you do implement it, rather than manually fetching protoc from a URL I would suggest instead declaring a new gradle configuration and adding a dependency in that configuration on the protoc maven artifact. Then the gradle task (from the previous point) can depend on that configuration and gradle will take care of downloading and caching for you.

[nbaztec]

@garyp I tried to implement as per your suggestions, please take a look. My Gradle competencies are quite limited so feel free to suggest changes.

[garyp]

This is exactly what I had in mind. There are a few minor things to address, but then this should be good to merge.

I do think it'd make more sense to have DescriptorProtocTask extend the existing ProtocTask. Looking at the code you have here, it'd only take minimal changes to ProtocTask in order to make it useable for this use case. However, in the interest of iterative development, we can merge this version after you address the other comments, and then refactor DescriptorProtocTask to extend ProtocTask in a future PR.

[garyp]

One minor nit, otherwise looks good.

[garyp]

To make sure the Gradle plugin versions are consistent across the entire project, we declare all plugins in the top-level build.gradle.kts file with apply false. Then we apply them in the individual Gradle subprojects as needed.

[garyp]

Thanks again for adding this!

[nbaztec]

I think the task compileTestJs has some issues. I've got it once before and re-running the build (with empty commit) worked:

> Task :runtime:compileTestKotlinJs
e: java.lang.StackOverflowError
	at org.jetbrains.kotlin.js.inline.clean.TemporaryVariableElimination$analyze$1.visitBinaryExpression(TemporaryVariableElimination.kt)
	at org.jetbrains.kotlin.js.backend.ast.JsBinaryOperation.accept(JsBinaryOperation.java:47)
	at org.jetbrains.kotlin.js.backend.ast.JsVisitor.accept(JsVisitor.kt:11)

Would it be fine if you re-ran it or should I add empty commits?

[garyp]

@nbaztec I'm rerunning it. We've been seeing a lot more StackOverflowErrors in the compiler with Kotlin 1.4 😞 Do you remember if the last time you saw this error it was also in the compileTestKotlinJs task?

[nbaztec]

I think yes, here's the build https://github.com/streem/pbandk/runs/1673410304