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 scripts to run interop-tests on grpc-web Java connector #869

Merged

Conversation

vnorigoog
Copy link
Collaborator

No description provided.

sleep 15

# Run interop tests
docker run --network=host --rm grpcweb/prereqs /bin/bash /github/grpc-web/scripts/docker-run-interop-tests.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nice you can run exactly the same interop client!!

docker run --network=host --rm grpcweb/prereqs /bin/bash /github/grpc-web/scripts/docker-run-interop-tests.sh

docker rm -f "$pid"

Copy link
Collaborator

@stanley-cheung stanley-cheung Jun 24, 2020

Choose a reason for hiding this comment

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

Can you put L42 - L49 above in here: https://github.com/grpc/grpc-web/blob/master/scripts/run_interop_tests.sh#L49

Then you don't need this run_java_interop_tests.sh script.

You can merge your L43 above with https://github.com/grpc/grpc-web/blob/master/scripts/run_interop_tests.sh#L34

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good idea!
done.
PTAL

Copy link
Collaborator

@stanley-cheung stanley-cheung left a comment

Choose a reason for hiding this comment

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

I am able to run the new run_interop_tests.sh successfully now! Awesome!

#
# Run interop tests against grpc-web java connector code
#
docker-compose build java-interop-server
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: can we merge this line with L34 above? So we build all the docker images first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

#
docker-compose build java-interop-server
pid3=$(docker run -d --network=host grpcweb/java-interop-server)
docker run --network=host --rm grpcweb/prereqs /bin/bash /github/grpc-web/scripts/docker-run-interop-tests.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: can we break this long line like L43-44 above? Just to be consistent.

Thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. (made a shell function to call tests)

@vnorigoog
Copy link
Collaborator Author

Thanks for the reviews. PTAL

@stanley-cheung
Copy link
Collaborator

Thanks for the reviews. PTAL

Thanks! Looks good. One final nit: can you please squash this down to maybe 1 commit please?

Copy link
Collaborator

@stanley-cheung stanley-cheung left a comment

Choose a reason for hiding this comment

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

Perfect! Now each time a PR is created, tests will be run against the Java interop server.

@stanley-cheung stanley-cheung merged commit 74ea9b3 into grpc:master Jun 25, 2020
@stanley-cheung stanley-cheung changed the title add scripts etc to run interop-tests on grpc-web java connector code Add scripts to run interop-tests on grpc-web Java connector Jun 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants