Auto-discover protocoltest instead of manually adding them to build#3392
Auto-discover protocoltest instead of manually adding them to build#3392Madrigal wants to merge 6 commits into
Conversation
| @@ -0,0 +1,43 @@ | |||
| # Protocol Test Codegen – Manual Files | |||
There was a problem hiding this comment.
nit: why we need 2 same README here?
There was a problem hiding this comment.
It's an artifact of manually copying all files from manual to here. There's some discussion about just not checking them in, which may be the way we go instead.
| @@ -1,240 +1,281 @@ | |||
| { | |||
| "version": "1.0", | |||
There was a problem hiding this comment.
The sdk-codegen project does the same programmatic generation but we don't check in the file at all, we should do the same here, mainly so someone doesn't edit this one by mistake (but also we just don't need it).
There was a problem hiding this comment.
Ah, this sounds good to me, will remove it and just keep it as part of generate
There was a problem hiding this comment.
Removed it from checking it in, but it is kind of annoying that every time I run generate the file is there preventing me from switching to a different branch if it exists on that branch.
I could add it to .gitignore, but I'm not sure if that would cause any issue with workflows that expect the file to exist.
| @@ -0,0 +1,43 @@ | |||
| # Protocol Test Codegen – Manual Files | |||
There was a problem hiding this comment.
We might want to just have a GoIntegration that generates this stuff instead of having to have this new mechanism but it's nbd. I imagine it could just gate on "is service in protocoltests namespace" etc.
There was a problem hiding this comment.
This sounds good, but if we have a GoIntegration writing manual files will mean writing a Java file that creates a Go template, right?
8b466d4 to
b5585d1
Compare
Currently, protocol tests are created by manually updating codegen/protocol-test-codegen/smithy-build.json, where we manually input which service to use according to a template.
This means when a new protocol test gets added, we need to read the release notes and remember to manually add them to this file.
With this PR, protocol tests are instead read from the runtime class path. We find all Smithy services available and generate the
smithy-build.jsonfile manually. This is done on as a new stepgenerate-smithy-buildon protocol test gradle file which is attached to thebuildstep, which now generates the whole content ofinternal/protocoltest.This PR leaves existing tests exactly as-is, name and everything. The diff of
smithy-build.jsononly indicates changes on position, but all entries are the same.Removing services in this script instead of ProtocolUtils in smithy-go since we're removing whole services instead of just individual tests.
Added a step to add manual files to
internal/protocoltestsince there are some tests that we wrote manually and would be lost.