Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

chore: update showcase version for sequence tests #3268

Merged
merged 8 commits into from
Aug 25, 2020

Conversation

noahdietz
Copy link
Contributor

@noahdietz noahdietz commented Aug 24, 2020

This updates the version of Showcase used for generation in CI to v0.12.0 and adds a test for the new SequenceService to assert the basic retry attempt logic and timeout-backoff behavior works as implemented.

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 24, 2020
@noahdietz noahdietz force-pushed the showcase-sequence-test branch from 3594e1c to 4cde440 Compare August 24, 2020 21:24
@noahdietz
Copy link
Contributor Author

I'm not really sure why the ruby tests are failing with TypeError: Sequence is not a module - my hunch is that the service is named SequenceService and there is a message named Sequence and somehow this results in a naming conflict.

Not really sure what to do about that. We've migrated everything to gapic-generator-ruby, maybe we shouldn't make the tests required?

Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

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

LGTM with a a few minor comments.
Let's ask @viacheslav-rostovtsev he things about ruby failures?

@noahdietz
Copy link
Contributor Author

Just spoke to dazuma and he said that naming collisions are definitely possible, real cases in cloud libs were worked around with synthtool, and were ultimately fixed with the microgenerator. I guess I should do something to exclude this proto from Ruby generation in this CI.

@noahdietz
Copy link
Contributor Author

Fixed it by breaking out ruby generation, and before generating it, I moved the offending proto out of the target directory and remove the service from the 1P yaml. Hacky, but really the only way to exclude this proto from ruby showcase gen.

@codecov
Copy link

codecov bot commented Aug 25, 2020

Codecov Report

Merging #3268 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3268   +/-   ##
=========================================
  Coverage     87.12%   87.12%           
  Complexity     6080     6080           
=========================================
  Files           494      494           
  Lines         24060    24060           
  Branches       2613     2613           
=========================================
  Hits          20962    20962           
  Misses         2236     2236           
  Partials        862      862           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc019bd...8c653b5. Read the comment docs.

@noahdietz noahdietz merged commit 6c4b62a into googleapis:master Aug 25, 2020
@noahdietz noahdietz deleted the showcase-sequence-test branch August 25, 2020 15:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants