8000 Add interop test spec and interop tests by stanley-cheung · Pull Request #772 · grpc/grpc-web · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add interop test spec and interop tests #772

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 27, 2020
Merged

Add interop test spec and interop tests #772

merged 5 commits into from
Apr 27, 2020

Conversation

stanley-cheung
Copy link
Collaborator

This PR adds an interop test specifications to this repo, in the hopes that we have a uniform way to test different implementation and integration with different gRPC-Web components.

   gRPC-Web client   <--->   Proxy   <--->   gRPC Interop Service

The idea is that we should be able to swap out any of these 3 components and the set of gRPC-Web interop tests should still pass.

The client can be any client that implements the grpc-web spec, including a curl client for example. In the middle we can swap out Envoy with the Java in-process proxy we are currently working on, for example. The gRPC Interop Service can be any server that implements the interop test suite in any supported gRPC languages.

The canonical set of tests are based on the specification found in the grpc/grpc repo. Here in gRPC-Web only a subset of the tests will be relevant to us (e.g. we won't be implementing any tests involving client-streaming and bidi-streaming). We will also try to add tests that's specific to gRPC-Web.

This PR also starts to add the interop test implementations for this reference repo. It's not complete yet - there will be more work on this. In fact, we have identified some issues because of these tests so we need to fix some of those issues in the library first.

@stanley-cheung
Copy link
Collaborator Author
stanley-cheung commented Apr 2, 2020 •
edited
Loading

Some TODOs and TBDs:

  • Figure out what's the best way the interop test should be run for the JS client in this repo. Right now all the tests are run serially when you open up the browser app. In most interop client, there is an option to run one specific test case. Also, having to open up a browser app is pretty manual - figure out how we can automate this better.
    • Done. The interop tests can now be run as npm test and has also been integrated with the main kokoro.sh entry script, provided that the interop server and Envoy are running.
  • Should we have written the tests with Promises? Or should we be using some sort of testing frameworks?
    • Done. Using mocha and plain old require('assert')
  • Figure out whether / how we can implement those tests involving compression.
  • Figure out whether / how we can implement those tests involving auth. Perhaps the set of grpc-web interop tests involving auth have to be different from the canonical tests from grpc.
  • Add some tests that swap out the client with curl.
  • Figure out whether / how we can implement those tests involving --use_tls.
  • For tests involving callbacks, figure out what's the best way to assert that several different callbacks had been called in an async way. E.g. if we expect the on('data', ...) callback to be called 4 times, how do we know that we have to fail the test case if the callback had been called 3 times. And if we have to assert across different callbacks, at what point do we resolve()?
    • Done. mocha has a mechanism with calling done() in async callbacks.
  • Add tests to test binary mode in addition to text mode
    • Done. Now tests are both run on text mode and binary mode.

@stanley-cheung stanley-cheung force-pushed the add-interop branch 3 times, most recently from 02bc4c9 to 8d27522 Compare April 4, 2020 00:25
@jtattermusch
Copy link
Contributor

CC @JamesNK @JunTaoLuo

jtattermusch
jtattermusch reviewed Apr 6, 2020
View reviewed changes
Copy link
Contributor
@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

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

The initial version of interop-test-descriptions.md looks good, @JamesNK may have some questions about implementing this.

I haven't checked the .js code at all (not very familar with the language).

Besides that test definitions, we will also need some simple test harness for actually running the tests - that can come in a followup PR.

@JamesNK
Copy link
Member
JamesNK commented Apr 6, 2020 
   gRPC-Web client   <--->   Proxy   <--->   gRPC Interop Service

For grpc-dotnet:

  • We have a .NET gRPC-Web client. It would swap out the gRPC-Web client.
  • We have an in-process .NET gRPC-Web proxy. It would swap out 8000 the proxy and the gRPC interop service.

Questions:

  • How should grpc-dotnet package the client/server so they can easily be run? Docker?
  • Is there any plan to automate running implementation interop tests together?

@stanley-cheung
Copy link
Collaborator Author

Questions:

  • How should grpc-dotnet package the client/server so they can easily be run? Docker?

Yes that would be preferred. I am planning the same for the canonical implementation.

  • Is there any plan to automate running implementation interop tests together?

Yes. But that may come later. We have 3 sets of tests in the foreseeable future: this canonical JS+Envoy test, your dotnet client/server, the Java in-process proxy we are working on. We are certainly looking to automate this. We have an entry point scripts/kokoro.sh in this repo that kicks off a bunch of tests to be run every time a PR is created. So I will be looking into integrating these interop tests there.

@stanley-cheung
Copy link
Collaborator Author

For grpc-dotnet:

  • We have a .NET gRPC-Web client. It would swap out the gRPC-Web client.
  • We have an in-process .NET gRPC-Web proxy. It would swap out the proxy and the gRPC interop service.

I updated the PR to make it clearer that

  • for new gRPC-Web clients, you need to swap out the JS client we provided and keep using Envoy to pass the tests
  • for in-process proxies, you need to swap out Envoy+Backend as a unit

Thanks for pointing that out.

@JamesNK
Copy link
Member
JamesNK commented Apr 8, 2020

Yes. But that may come later.

That's fine. I'd like to think about some technical aspects of it now.

With the standard gRPC tests a client process is started for each test, and success or failure is reported by the exit code of the client process.

At the moment this PR has the canonical gRPC-Web client tests are being run in a browser together. That makes figuring out programmatically what tests were successful and what failed difficult. Should a proper JS test framework be used?

Related: There is a .NET gRPC-Web client, and I was concerned about how it could fit into a browser based test system. I think that can easily be solved by having the JS test framework call into a Blazor WebAssembly app, and have it execute the test logic there, and then return back to the JS test method with the result.

@stanley-cheung
Copy link
Collaborator Author

At the moment this PR has the canonical gRPC-Web client tests are being run in a browser together. That makes figuring out programmatically what tests were successful and what failed difficult. Should a proper JS test framework be used?

Yes that's definitely my plan - see the first item in my TODO list earlier in this thread. I definitely want to somehow automate this using a framework so that we don't need to manually open up a browser.

I don't think opening up a browser to run the test is necessary for this. As long as the testing framework is supposed to mimic running the test on a browser (I actually wasn't very familiar with this ecosystem so I need to do more research on it. If you find anything please let me know), I absolutely would like to be able to run it from the command line.

@JamesNK
Copy link
Member
JamesNK commented Apr 8, 2020

Opps, I didn't read your todo list.

Last time I asked about JavaScript testing (1.5 years ago) the most common suggestion was https://jestjs.io/

@JamesNK
Copy link
Member
JamesNK commented Apr 8, 2020

I looked at the grpc-node repository and it uses mocha which is another popular choice.

JamesNK
JamesNK reviewed Apr 9, 2020
View reviewed changes
interop-test-descriptions.md
| google_default_credentials | TBD | TBD |
| compute_engine_channel_credentials | TBD | TBD |
| custom_metadata | &#10003; | &#10003; |
| status_code_and_message | &#10003; | &#10003; |
Copy link
Member

Choose a reason for hiding this comment

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

status_code_and_message has a full duplex call.

https://github.com/grpc/grpc/blob/f12df974aeb305d3fc322b2b3aaab38d46b0c30a/doc/interop-test-descriptions.md#status_code_and_message

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea I probably should put an asterisk in those tests where we should only implement the UnaryCall part.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is the only one like that with a tick.

@JamesNK
Copy link
Member
JamesNK commented Apr 9, 2020 •
edited
Loading

gRPC-Web client and Docker containers:

grpc/grpc-dotnet#859

I've added a readme with instructions on how to run the gRPC-Web server and client containers.

FYI, I have the client_compressed_unary, server_compressed_unary and server_compressed_streaming passing with the .NET gRPC-Web client.

@stanley-cheung
Copy link
Collaborator Author

Some updates:

  • I used an npm package "xhr2" to swap out the XHR runtime and is able to automate running the interop tests with an npm test command. This has been integrated with our main kokoro.sh script.
  • So now we don't need to bring up a browser to run these tests. The tests are written with mocha and simple assert statements
  • Both grpc-web-text and grpc-web+protoc (binary) mode are being tested now

@stanley-cheung stanley-cheung merged commit 1dffd21 into grpc:master Apr 27, 2020
@stanley-cheung stanley-cheung deleted the add-interop branch April 27, 2020 23:27
@johanbrandhorst
Copy link
Collaborator

This is great! We should port the implementations from https://github.com/johanbrandhorst/grpc-web-compatibility-test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamesNK JamesNK JamesNK left review comments

@jtattermusch jtattermusch jtattermusch left review comments

Assignees
No one assigned
Labels
release notes: major
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stanley-cheung @jtattermusch @JamesNK @johanbrandhorst
0