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

Remove stale target tests #3770

Closed
wants to merge 2 commits into from

Conversation

tybug
Copy link
Member

@tybug tybug commented Oct 15, 2023

I'm taking a look at improving cover test times. test_targeting_with_following_empty takes 3-5 seconds to run - target(len(lst)) is dangerous, as my observation is hypothesis occasionally tries doubling the length of the list, which can quickly get out of hand and cause slow generations when unbounded.

I was going to simply improve the runtime. However, I don't think the test (and the related test_targeting_with_many_empty) is needed anymore. They were introduced in #2137, back when there was special-case handling for empty vs non-empty examples in Optimiser. This logic was changed in #2289, and I don't think there's a distinction between empty and non-empty anymore, but please correct me if there is.

@Zac-HD
Copy link
Member

Zac-HD commented Oct 15, 2023

I've cherry-picked this into #3769 with some other minor fixes, and queued it to merge and release 🎉

Thanks @tybug!

@Zac-HD Zac-HD closed this Oct 15, 2023
@tybug tybug deleted the remove-stale-target-tests branch October 15, 2023 21:35
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