Skip to content
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
wants to merge 39 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
30cd85d
Fix typos
vchudnov-g Sep 27, 2019
eaec552
Bare-bones scaffold for acceptance harness
vchudnov-g Sep 27, 2019
7b3e100
Move most logging to a debug logger
vchudnov-g Sep 27, 2019
9312158
Iterate over all suite config dirs. Add a simple suite config.
vchudnov-g Sep 27, 2019
1ba8c2c
Check needed dependencies. Add script to prepare them.
vchudnov-g Sep 28, 2019
f0abe48
Start and stop gapic-showcase in the background.
vchudnov-g Sep 28, 2019
a65893d
Build gapic-showcase from local tree.
vchudnov-g Sep 28, 2019
4fac512
Add some comments/TODos
vchudnov-g Sep 30, 2019
54ddeb5
use go-trace package for debug statements
vchudnov-g Sep 30, 2019
1139617
Classify suite files by type
vchudnov-g Oct 2, 2019
8a339c3
Add instructions for running protoc w Go plugin
vchudnov-g Oct 3, 2019
d09898f
Use packr to get files from disk
vchudnov-g Oct 3, 2019
c49b73d
Rename "Suite" to "Scenario"
vchudnov-g Oct 3, 2019
3baf0ac
Add some TODOs
vchudnov-g Oct 4, 2019
feec344
Create sandbox directory with a per-run timestamp in its name
vchudnov-g Oct 4, 2019
683c0ff
WIP: Running protoc
vchudnov-g Oct 5, 2019
73b3768
Make the file list stored with each scenario be relative paths
vchudnov-g Oct 5, 2019
60456c2
Cosmetic changes
vchudnov-g Oct 5, 2019
ff2b117
Get common proto files in a box
vchudnov-g Oct 7, 2019
508e6c2
Process top-level .include files and directories
vchudnov-g Oct 8, 2019
85188ef
Allow include files anywhere in acceptance/ hierarchy into sandbox
vchudnov-g Oct 8, 2019
1389599
Better trace the assets read in
vchudnov-g Oct 8, 2019
2acf5c1
Check the generation step succeded via zero exit code
vchudnov-g Oct 9, 2019
238659c
Make the generation step run with samplegen turned on
vchudnov-g Oct 11, 2019
27f00d9
Move qualify to be a gapic-showcase subcommand
vchudnov-g Oct 11, 2019
b773222
Remove debugging comments
vchudnov-g Oct 11, 2019
8fbe238
Allow passing flags and args to the qualify command
vchudnov-g Oct 11, 2019
242179f
Run showcase servers in process when qualifying
vchudnov-g Oct 11, 2019
d34b2aa
Check for python plugin in prepare-to-qualify
vchudnov-g Oct 12, 2019
44447c3
Make Packr part of the release process.
vchudnov-g Oct 12, 2019
34cb76b
Remove startShowcase() and endProcess()
vchudnov-g Oct 14, 2019
9265b41
Add sanity check in case the assets are empty
vchudnov-g Oct 14, 2019
36a69f8
Remove getAllFiles
vchudnov-g Oct 14, 2019
25eb747
Document code and add licenses
vchudnov-g Oct 14, 2019
c80a225
Update go.mod and go.sum after rebasing to latest main/HEAD
vchudnov-g Oct 14, 2019
a7c1bf7
Rename the demo test suite so it's called "basic-check"
vchudnov-g Oct 14, 2019
a41706a
Minor cosmetic tweaks
vchudnov-g Oct 15, 2019
46f760f
Code review changes: code
vchudnov-g Oct 21, 2019
fe47602
Give the example includes more descriptive names
vchudnov-g Oct 21, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Temp directories
tmp/
dist/
**/packrd/*
**/*-packr.go
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ hold using this installation method._

## Schema
The schema of GAPIC Showcase API can be found in [schema/google/showcase/v1beta1](schema/google/showcase/v1beta1)
It's dependencies can be found in the [googleapis/api-common-protos](https://github.com/googleapis/api-common-protos)
Its dependencies can be found in the [googleapis/api-common-protos](https://github.com/googleapis/api-common-protos)
submodule.

## Quick Start
Expand Down
210 changes: 210 additions & 0 deletions cmd/gapic-showcase/qualifier/acceptance_suite/basic-check/echo.proto
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";
Copy link
Contributor Author

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.

Copy link
Collaborator

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!

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

additional instances of the file should have just a trivial cost

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 see echo.proto and assume a lot that might actually no longer be true for an individual copy.

we could look into being able to include showcase API protos via include files

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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.

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

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.


// 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;
}
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
api-common-protos/google/
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
api-common-protos/README.md
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
Loading