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

Improve ability to generate duplicated values #2261

Merged
merged 9 commits into from
Dec 18, 2019
Merged

Conversation

DRMacIver
Copy link
Member

I'm trying to slim down GenerationParameters a bit because it's rather too mysterious in operation (despite being an improvement on what came before!), so I wanted to get rid of the alphabet code. This caused a small number of tests to fail, more or less as expected.

The only major benefit of the alphabet code is that it helps us generate duplicates. So this PR adds a smol mutation step to generation that introduces duplication. This works better than the alphabet for creating duplicates because it also works for larger examples.

This did cause one test to fail which I just deleted, which was that it was possible to have all feature flags disabled. I thought about fixing that, and then I asked myself why that was a thing that we actually ever wanted to be possible, so I just deleted the test instead.

@DRMacIver
Copy link
Member Author

(Sorry just realised I got distracted midway through commenting this code, so I'm going to do that now)

@Zac-HD Zac-HD added the internals Stuff that only Hypothesis devs should ever see label Dec 5, 2019
Comment on lines 654 to 657
# Rather than make this the responsibility of individual strategies
# we implement a small mutator that just takes parts of the test
# case with the same label and tries replacing one of them with a
# copy of the other.
Copy link
Contributor

Choose a reason for hiding this comment

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

This will sound obvious, but I think this comment should explicitly state that we actually run the mutated data immediately after creating it.

That way the reader is more likely to look for (and find) the cached_test_function call that appears below.

@DRMacIver
Copy link
Member Author

The test failure should stop once #2262 is merged and this is rebased on top of it.

@DRMacIver DRMacIver force-pushed the DRMacIver/duplication branch from 75cfb04 to 902ac47 Compare December 16, 2019 21:49
Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

Looks good to me, though @Zalathar might also have some comments?

@DRMacIver DRMacIver merged commit 1e119b6 into master Dec 18, 2019
@Zac-HD Zac-HD deleted the DRMacIver/duplication branch December 18, 2019 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internals Stuff that only Hypothesis devs should ever see
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants