-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial implementation of GAPIC generator acceptance harness #222
Draft
vchudnov-g
wants to merge
39
commits into
googleapis:main
Choose a base branch
from
vchudnov-g:sample-acceptance
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 37 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
30cd85d
Fix typos
vchudnov-g eaec552
Bare-bones scaffold for acceptance harness
vchudnov-g 7b3e100
Move most logging to a debug logger
vchudnov-g 9312158
Iterate over all suite config dirs. Add a simple suite config.
vchudnov-g 1ba8c2c
Check needed dependencies. Add script to prepare them.
vchudnov-g f0abe48
Start and stop gapic-showcase in the background.
vchudnov-g a65893d
Build gapic-showcase from local tree.
vchudnov-g 4fac512
Add some comments/TODos
vchudnov-g 54ddeb5
use go-trace package for debug statements
vchudnov-g 1139617
Classify suite files by type
vchudnov-g 8a339c3
Add instructions for running protoc w Go plugin
vchudnov-g d09898f
Use packr to get files from disk
vchudnov-g c49b73d
Rename "Suite" to "Scenario"
vchudnov-g 3baf0ac
Add some TODOs
vchudnov-g feec344
Create sandbox directory with a per-run timestamp in its name
vchudnov-g 683c0ff
WIP: Running protoc
vchudnov-g 73b3768
Make the file list stored with each scenario be relative paths
vchudnov-g 60456c2
Cosmetic changes
vchudnov-g ff2b117
Get common proto files in a box
vchudnov-g 508e6c2
Process top-level .include files and directories
vchudnov-g 85188ef
Allow include files anywhere in acceptance/ hierarchy into sandbox
vchudnov-g 1389599
Better trace the assets read in
vchudnov-g 2acf5c1
Check the generation step succeded via zero exit code
vchudnov-g 238659c
Make the generation step run with samplegen turned on
vchudnov-g 27f00d9
Move qualify to be a gapic-showcase subcommand
vchudnov-g b773222
Remove debugging comments
vchudnov-g 8fbe238
Allow passing flags and args to the qualify command
vchudnov-g 242179f
Run showcase servers in process when qualifying
vchudnov-g d34b2aa
Check for python plugin in prepare-to-qualify
vchudnov-g 44447c3
Make Packr part of the release process.
vchudnov-g 34cb76b
Remove startShowcase() and endProcess()
vchudnov-g 9265b41
Add sanity check in case the assets are empty
vchudnov-g 36a69f8
Remove getAllFiles
vchudnov-g 25eb747
Document code and add licenses
vchudnov-g c80a225
Update go.mod and go.sum after rebasing to latest main/HEAD
vchudnov-g a7c1bf7
Rename the demo test suite so it's called "basic-check"
vchudnov-g a41706a
Minor cosmetic tweaks
vchudnov-g 46f760f
Code review changes: code
vchudnov-g fe47602
Give the example includes more descriptive names
vchudnov-g File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
# Temp directories | ||
tmp/ | ||
dist/ | ||
**/packrd/* | ||
**/*-packr.go |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
210 changes: 210 additions & 0 deletions
210
cmd/gapic-showcase/qualifier/acceptance_suite/basic-check/echo.proto
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,210 @@ | ||
// Copyright 2018 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// https://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
syntax = "proto3"; | ||
|
||
import "google/api/annotations.proto"; | ||
import "google/api/client.proto"; | ||
import "google/api/field_behavior.proto"; | ||
import "google/longrunning/operations.proto"; | ||
import "google/protobuf/duration.proto"; | ||
import "google/protobuf/timestamp.proto"; | ||
import "google/rpc/status.proto"; | ||
|
||
package google.showcase.v1beta1; | ||
|
||
option go_package = "github.com/googleapis/gapic-showcase/server/genproto"; | ||
option java_package = "com.google.showcase.v1beta1"; | ||
option java_multiple_files = true; | ||
|
||
// This service is used showcase the four main types of rpcs - unary, server | ||
// side streaming, client side streaming, and bidirectional streaming. This | ||
// service also exposes methods that explicitly implement server delay, and | ||
// paginated calls. Set the 'showcase-trailer' metadata key on any method | ||
// to have the values echoed in the response trailers. | ||
service Echo { | ||
// This service is meant to only run locally on the port 7469 (keypad digits | ||
// for "show"). | ||
option (google.api.default_host) = "localhost:7469"; | ||
|
||
// This method simply echos the request. This method is showcases unary rpcs. | ||
rpc Echo(EchoRequest) returns (EchoResponse) { | ||
option (google.api.http) = { | ||
post: "/v1beta1/echo:echo" | ||
body: "*" | ||
}; | ||
} | ||
|
||
// This method split the given content into words and will pass each word back | ||
// through the stream. This method showcases server-side streaming rpcs. | ||
rpc Expand(ExpandRequest) returns (stream EchoResponse) { | ||
option (google.api.http) = { | ||
post: "/v1beta1/echo:expand" | ||
body: "*" | ||
}; | ||
// TODO(landrito): change this to be `fields: ["content", "error"]` once | ||
// github.com/dcodeIO/protobuf.js/issues/1094 has been resolved. | ||
option (google.api.method_signature) = "content,error"; | ||
} | ||
|
||
// This method will collect the words given to it. When the stream is closed | ||
// by the client, this method will return the a concatenation of the strings | ||
// passed to it. This method showcases client-side streaming rpcs. | ||
rpc Collect(stream EchoRequest) returns (EchoResponse) { | ||
option (google.api.http) = { | ||
post: "/v1beta1/echo:collect" | ||
body: "*" | ||
}; | ||
} | ||
|
||
// This method, upon receiving a request on the stream, will pass | ||
// the same content back on the stream. This method showcases | ||
// bidirectional streaming rpcs. | ||
rpc Chat(stream EchoRequest) returns (stream EchoResponse); | ||
|
||
// This is similar to the Expand method but instead of returning a stream of | ||
// expanded words, this method returns a paged list of expanded words. | ||
rpc PagedExpand(PagedExpandRequest) returns (PagedExpandResponse) { | ||
option (google.api.http) = { | ||
post: "/v1beta1/echo:pagedExpand" | ||
body: "*" | ||
}; | ||
} | ||
|
||
// This method will wait the requested amount of and then return. | ||
// This method showcases how a client handles a request timing out. | ||
rpc Wait(WaitRequest) returns (google.longrunning.Operation) { | ||
option (google.api.http) = { | ||
post: "/v1beta1/echo:wait" | ||
body: "*" | ||
}; | ||
option (google.longrunning.operation_info) = { | ||
response_type: "WaitResponse" | ||
metadata_type: "WaitMetadata" | ||
}; | ||
} | ||
|
||
// This method will block (wait) for the requested amount of time | ||
// and then return the response or error. | ||
// This method showcases how a client handles delays or retries. | ||
rpc Block(BlockRequest) returns (BlockResponse) { | ||
option (google.api.http) = { | ||
post: "/v1beta1/echo:block" | ||
body: "*" | ||
}; | ||
}; | ||
} | ||
|
||
// The request message used for the Echo, Collect and Chat methods. If content | ||
// is set in this message then the request will succeed. If status is set in | ||
// this message then the status will be returned as an error. | ||
message EchoRequest { | ||
oneof response { | ||
// The content to be echoed by the server. | ||
string content = 1; | ||
|
||
// The error to be thrown by the server. | ||
google.rpc.Status error = 2; | ||
} | ||
} | ||
|
||
// The response message for the Echo methods. | ||
message EchoResponse { | ||
// The content specified in the request. | ||
string content = 1; | ||
} | ||
|
||
// The request message for the Expand method. | ||
message ExpandRequest { | ||
// The content that will be split into words and returned on the stream. | ||
string content = 1; | ||
|
||
// The error that is thrown after all words are sent on the stream. | ||
google.rpc.Status error = 2; | ||
} | ||
|
||
// The request for the PagedExpand method. | ||
message PagedExpandRequest { | ||
// The string to expand. | ||
string content = 1 [(google.api.field_behavior) = REQUIRED]; | ||
|
||
// The amount of words to returned in each page. | ||
int32 page_size = 2; | ||
|
||
// The position of the page to be returned. | ||
string page_token = 3; | ||
} | ||
|
||
// The response for the PagedExpand method. | ||
message PagedExpandResponse { | ||
// The words that were expanded. | ||
repeated EchoResponse responses = 1; | ||
|
||
// The next page token. | ||
string next_page_token = 2; | ||
} | ||
|
||
// The request for Wait method. | ||
message WaitRequest { | ||
oneof end { | ||
// The time that this operation will complete. | ||
google.protobuf.Timestamp end_time = 1; | ||
|
||
// The duration of this operation. | ||
google.protobuf.Duration ttl = 4; | ||
} | ||
|
||
oneof response { | ||
// The error that will be returned by the server. If this code is specified | ||
// to be the OK rpc code, an empty response will be returned. | ||
google.rpc.Status error = 2; | ||
|
||
// The response to be returned on operation completion. | ||
WaitResponse success = 3; | ||
} | ||
} | ||
|
||
// The result of the Wait operation. | ||
message WaitResponse { | ||
// This content of the result. | ||
string content = 1; | ||
} | ||
|
||
// The metadata for Wait operation. | ||
message WaitMetadata { | ||
// The time that this operation will complete. | ||
google.protobuf.Timestamp end_time =1; | ||
} | ||
|
||
// The request for Block method. | ||
message BlockRequest { | ||
// The amount of time to block before returning a response. | ||
google.protobuf.Duration response_delay = 1; | ||
|
||
oneof response { | ||
// The error that will be returned by the server. If this code is specified | ||
// to be the OK rpc code, an empty response will be returned. | ||
google.rpc.Status error = 2; | ||
|
||
// The response to be returned that will signify successful method call. | ||
BlockResponse success = 3; | ||
} | ||
} | ||
|
||
// The response for Block method. | ||
message BlockResponse { | ||
// This content can contain anything, the server will not depend on a value | ||
// here. | ||
string content = 1; | ||
} |
1 change: 1 addition & 0 deletions
1
cmd/gapic-showcase/qualifier/acceptance_suite/basic-check/foo/bar/include.wdREAD
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
api-common-protos/CONTRIBUTING.md | ||
noahdietz marked this conversation as resolved.
Show resolved
Hide resolved
|
1 change: 1 addition & 0 deletions
1
cmd/gapic-showcase/qualifier/acceptance_suite/basic-check/include.google
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
api-common-protos/google/ |
1 change: 1 addition & 0 deletions
1
cmd/gapic-showcase/qualifier/acceptance_suite/basic-check/include.vcREAD
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
api-common-protos/README.md |
31 changes: 31 additions & 0 deletions
31
cmd/gapic-showcase/qualifier/acceptance_suite/basic-check/sample.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
--- | ||
type: com.google.api.codegen.SampleConfigProto | ||
schema_version: 1.2.0 | ||
samples: | ||
- service: google.showcase.v1beta1.Echo | ||
rpc: Echo | ||
description: A sample of a simple echo request and response | ||
request: | ||
- field: content | ||
comment: The content to be echoed back | ||
value: a resounding echo...echo...echo | ||
input_parameter: message | ||
response: | ||
- comment: | ||
- "We simply print the response" | ||
- print: | ||
- 'Heard back: "%s"' | ||
- $resp.content | ||
--- | ||
type: test/sample | ||
schema_version: 1 | ||
--- | ||
type: inventory | ||
widget: "$expensive" | ||
--- | ||
elements: | ||
- hydrogen | ||
- oxygen | ||
--- | ||
type: sample/other | ||
tool: my_yaml |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking this file should become a symlink to the canonical
echo.proto
file, rather than a copy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 I was going to say the exact same thing!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, Packr does not include symlinked files....
I suggest for this PR, we leave this as is: an explicitly copied files. This makes it very explicit what's being tested. Packr does compress the files being included, so additional instances of the file should have just a trivial cost.
If we do become concerned about repeated files as we add to the acceptance suite (for example, if the repeated files get decompressed all at once rather than on-demand), we could look into being able to include showcase API protos via include files. But I suggest we defer that for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with doing this for now.
In this case, I'm not worried as much about size costs but rather maintenance costs. If we add a new annotation or one of the existing annotations changes, or we add a new RPC, we will be updating N
echo.proto
files and it is too easy to miss one. It is easy to grow out of sync with multiple copies and then our understanding of what is actually being tested is shaky i.e. we seeecho.proto
and assume a lot that might actually no longer be true for an individual copy.This should be one of the first things evaluated, especially as it might apply to api-common-protos && googleapis/googleapis. Checking in multiple copies of these sources will get out of hand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, we already have this for anything under
schema/
, but not for the showcase files themselves.You make a good argument for doing this for showcase files: not having to repeat the modification in all copies. If we don't do it, we'd find out pretty quickly which copies we missed updating because tests would fail---but that's a bad experience. If we do do it, it would make it a lot simpler to update the cases where we expect generation to succeed.
For cases where we expect generation to fail, depending on what we want the error to be, we may want to have copies rather than symlinks so that we can modify them to cause the error we want. For these cases, we may still run into the problem that if a newly required annotation is not present in the previously copied-and-modified file, the error message we get may be due to that instead of the cause we expected, and the test would thus fail. I don't think we can get around that other than by making sure to run the acceptance harness whenever a new annotation comes in, so we can update these cases manually if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's reasonable. But do they have to be a full copy of a showcase proto (e.g. echo.proto)? Can it just be a minimally working proto with just the one erroneous piece and have it named "gen_failure_{reason}.proto"? If we have 5 copies of echo.proto with entirely the same content except maybe a typo in an RPC name or 1 missing message in order to trigger a failure mode, it will be too easy to assume the wrong thing imo. Does that make sense? I just want the purpose of each added proto/input file to be very clear.
Yeah that's fair, it should be obvious when something is missing and a developer should expect to have to update everything in these scenarios.