Skip to content

chore(cts): assert the retry strategy error #3405

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

Merged
merged 14 commits into from
Jul 24, 2024
Merged

chore(cts): assert the retry strategy error #3405

merged 14 commits into from
Jul 24, 2024

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Jul 21, 2024

🧭 What and Why

Suggested in #3334, we can add tests for the retry strategy error message, even if its different for every language.

@millotp millotp self-assigned this Jul 21, 2024
@millotp millotp requested a review from a team as a code owner July 21, 2024 09:37
@millotp millotp requested review from morganleroi and shortcuts July 21, 2024 09:37
@millotp millotp marked this pull request as draft July 21, 2024 09:37
@algolia-bot
Copy link
Collaborator

algolia-bot commented Jul 21, 2024

✔️ Code generated!

Name Link
🪓 Triggered by 56193055de72b1e82a12d80db2898eeee08a02c5
🍃 Generated commit e8acc25335dedbfc2595fa0f718161d04035c378
🌲 Generated branch generated/chore/retry-error
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Rate
javascript 1349
php 1291
csharp 1019
java 938
ruby 849
swift 749
python 703
kotlin 477
go 463

morganleroi
morganleroi previously approved these changes Jul 22, 2024
Copy link
Contributor

@morganleroi morganleroi left a comment

Choose a reason for hiding this comment

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

Nice ! Thanks (a lot) for doing it.

@millotp millotp marked this pull request as ready for review July 23, 2024 15:55
Copy link

"path": "1/test/hang/${{language}}"
},
"expected": {
"error": {
Copy link
Member

Choose a reason for hiding this comment

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

if we add a new language, will this throw? I did not saw this specific case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes the CTS will add a default message and the test will fail, so its necessary to add it here

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

gg!!!!!!!!

@millotp millotp merged commit e974ffc into main Jul 24, 2024
21 checks passed
@millotp millotp deleted the chore/retry-error branch July 24, 2024 08:10
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.

5 participants