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

Add conformance client for connect-web #952

Closed
wants to merge 53 commits into from
Closed

Conversation

srikrsna-buf
Copy link
Member

@srikrsna-buf srikrsna-buf commented Dec 1, 2023

Add conformance client for connect-web. Uses webdriver to start a browser session adn run the conformance runner piping the output of the client-under-test back to the webdriver test and using execute to run the client on the browser.

Add a local target for chrome and firefox. I'll followup with browserstack config once this approach is finalised.

@srikrsna-buf srikrsna-buf changed the base branch from main to conformance/rename December 1, 2023 19:06
@srikrsna-buf srikrsna-buf marked this pull request as ready for review December 4, 2023 12:00
Base automatically changed from conformance/rename to main December 7, 2023 08:26
@srikrsna-buf
Copy link
Member Author

@timostamm Added a readme section describing the flow. It doesn't seem to render on GitHub for some reason, but renders on my machine. Will look into it. prettier also seems to have changed which changed the formatting a little bit

@timostamm
Copy link
Member

@timostamm Added a readme section describing the flow. It doesn't seem to render on GitHub for some reason, but renders on my machine. Will look into it.

We shouldn't need a diagram to explain how we run tests, and I'd much prefer a simple setup similar to the the gRPC-Web client tests here.

I think it's worth taking another look at webdriver.io's standalone mode. It takes over stdin/stdout, preventing us from using stdio with the test runner. But it seems quite odd that it does, and we might be able to provide an upstream fix or use a local workaround. The browser output is useful, but it would be sufficient to dump it into a log file. That would bring us much closer to the simplicity of the gRPC-Web client tests.

It's a possibility that we get it to work, and integration with browserstackbreaking it again. So I'm okay with merging what we have now, and look for a better solution after integration with browserstack.

FYI, puppeteer can instrument FF as well, so we do have options.

@@ -0,0 +1,71 @@
{
"name": "@connectrpc/connect-node-conformance",
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't have a lock file here, there should only be a single one at the repo root. This may have been caused by running npm install in this package.

@@ -10,5 +10,5 @@ it like a web server would usually do.

| code generator | bundle size | minified | compressed |
|----------------|-------------------:|-----------------------:|---------------------:|
| connect | 117,734 b | 51,217 b | 13,697 b |
| connect | 122,099 b | 53,623 b | 14,402 b |
Copy link
Member

Choose a reason for hiding this comment

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

This could be caused by new version of esbuild. We have to update esbuild separately, so that we have a good understanding of bundle size changes.

We'll update deps tomorrow - hopefully this (and changes from prettier) will automatically resolve.

@srikrsna-buf
Copy link
Member Author

Closing in favor of #1019

@srikrsna-buf srikrsna-buf deleted the conformance/web branch April 22, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants