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 some Python client tests #1441

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Conversation

anlambert
Copy link
Contributor

This PR adds some unit tests for the Svix Python OpenAPI client to avoid breaking things (see #1440).

This is still a work in progress, I just added a couple of simple tests but one of them showcases the issue mentioned above (see Github Actions job report for newly added workflow).

Copy link
Member

@svix-jplatte svix-jplatte left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR!

We'd be glad to have more unit tests, so please do add anything that you feel makes sense. Doesn't all have to be one PR either, but of course I'll honor your preference w.r.t. PR setup and will wait for the draft state to be removed before merging.

Also, I just noticed when reviewing the workflow file that there's some weird thing in there that you seem to have copied from our existing python lint workflows. Would you mind applying the changes from #1445 to the new workflow file? Specifically, I'm talking about the workflow trigger rules, and the version of the setup-python action.

@anlambert
Copy link
Contributor Author

We'd be glad to have more unit tests, so please do add anything that you feel makes sense. Doesn't all have to be one PR either, but of course I'll honor your preference w.r.t. PR setup and will wait for the draft state to be removed before merging.

Great, I do not intend to cover all the Python OpenAPI code but adding a couple of simple tests on the most used features (notably sending messages) should be sufficient to avoid bad surprises in the future. I will try to finish that work y the end of the week.

Also, I just noticed when reviewing the workflow file that there's some weird thing in there that you seem to have copied from our existing python lint workflows. Would you mind applying the changes from #1445 to the new workflow file? Specifically, I'm talking about the workflow trigger rules, and the version of the setup-python action.

Sure.

@anlambert anlambert force-pushed the python-client-tests branch 3 times, most recently from 3f174ad to 5075885 Compare September 30, 2024 17:55
@svix-jplatte
Copy link
Member

I see you force-pushed a few times, is this still work in progress?

@anlambert
Copy link
Contributor Author

I see you force-pushed a few times, is this still work in progress?

Sorry, I got delayed on this but I think it is ready for review.

This PR does not cover the whole Svix Python OpenAPI client features but I think it should be enough to catch any important breakages that could be introduced.

I am going to rebase it to the main branch and mark it ready for review.

@anlambert anlambert marked this pull request as ready for review October 2, 2024 12:04
@anlambert anlambert requested a review from a team as a code owner October 2, 2024 12:04
@anlambert
Copy link
Contributor Author

I am going to rebase it to the main branch and mark it ready for review.

Done !

Copy link
Member

@svix-jplatte svix-jplatte left a comment

Choose a reason for hiding this comment

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

Thanks!

@svix-jplatte svix-jplatte merged commit 034a6a4 into svix:main Oct 2, 2024
4 checks passed
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