-
Notifications
You must be signed in to change notification settings - Fork 22
ci: adding testing for v1 protos #2389
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
Conversation
added pytest fixture
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2389 +/- ##
==========================================
+ Coverage 95.11% 95.21% +0.09%
==========================================
Files 168 168
Lines 11290 11290
==========================================
+ Hits 10739 10750 +11
+ Misses 551 540 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm couple of things..
- Do we really need the stable/unstable logic for the v1 tests? I would simply run against the latest stable
- It might be easier for you to look at minimum requirements stages or the no graphics stages, rather than the main testing stages -- the v1 testing should be simpler I believe
- I really like what you did with the
proto_versionfixture. That way it also forces v0 input to work properly as well.
testing against python 3.10 and 3.13 only
RobPasMue
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - although it's going to be very annoying to see all the workflows fail constantly..
What if we create a separate workflow file, like the one we have for backwards compatibility. And only have it run on Pull Requests or on demand (workflow_dispatch) for now?
RobPasMue
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good - let's merge this!
Description
Issue linked
Testing related to #2388
Checklist
feat: extrude circle to cylinder)