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

Move maturin test coverage into CI #3714

Merged
merged 1 commit into from
May 21, 2024
Merged

Move maturin test coverage into CI #3714

merged 1 commit into from
May 21, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented May 21, 2024

This test can take over 60s to run, which is too much for a unit test. We'll run it in a separate CI job to retain coverage.

@zanieb zanieb added the testing Internal testing of behavior label May 21, 2024
@zanieb zanieb force-pushed the zb/maturin-test branch 3 times, most recently from 3cf58cc to 84d073d Compare May 21, 2024 18:42
@zanieb zanieb marked this pull request as ready for review May 21, 2024 18:46
@zanieb zanieb requested review from konstin and ibraheemdev May 21, 2024 18:46
@charliermarsh
Copy link
Member

Can we just remove this entirely? Seems like a lot of complexity to have an maintain job just for this.

@zanieb zanieb force-pushed the zb/maturin-test branch from 84d073d to 67b3ad0 Compare May 21, 2024 19:07
@zanieb
Copy link
Member Author

zanieb commented May 21, 2024

I don't feel strongly, we could remove it. Why did @konstin think coverage was important?

@zanieb zanieb force-pushed the zb/maturin-test branch from 67b3ad0 to f5a3dca Compare May 21, 2024 19:09
@zanieb
Copy link
Member Author

zanieb commented May 21, 2024

My goal here is immediately unblock the rest of my PRs which are failing due to this test. It's trivial to remove after this merges so I have a preference to merge this as-is and consider dropping coverage separately.

@zanieb zanieb added the internal A refactor or improvement that is not user-facing label May 21, 2024
@charliermarsh
Copy link
Member

Ok, feel free to merge, I'll remove in a separate PR then. Bias towards action :)

@zanieb zanieb enabled auto-merge (squash) May 21, 2024 19:14
@zanieb zanieb merged commit dfd6ccf into main May 21, 2024
45 checks passed
@zanieb zanieb deleted the zb/maturin-test branch May 21, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal A refactor or improvement that is not user-facing testing Internal testing of behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants