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

Test 0-length-arrays in fuzzing tests #37823

Merged
merged 3 commits into from
Dec 3, 2016

Conversation

danwinship
Copy link
Contributor

While hacking on #37289 I noticed that our fuzzing tests test nil slices and slices of length 1, but not slices of length 0, meaning we aren't testing that 0-length slices get treated the same as nil in all the places we expect them to (and in particular, we aren't ensuring that comparisons always use api.Semantic.DeepEqual rather than reflect.DeepEqual). (Though in fact, changing the fuzzer didn't turn up any bugs, so maybe this effectively gets tested somewhere else...)

fuzz.New().NilChance(.5).NumElements(0, 1) means we end up generating nil 50% of the time, a length 0 array 25% of the time, and a length 1 array 25% of the time... maybe it should be fuzz.New().NilChance(.33).NumElements(0, 1) instead?

The gofuzz rebase is to pull in google/gofuzz#20, and the other fix is just a drive-by.

Trying to create IntStrs from int64s rather than int32s resulted in
lots of glog.Error messages that showed up when running tests with
"-v".
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 1, 2016
@k8s-oncall
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Dec 1, 2016
@smarterclayton smarterclayton added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Dec 2, 2016
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 38049, 37823, 38000, 36646)

@k8s-github-robot k8s-github-robot merged commit 723a200 into kubernetes:master Dec 3, 2016
@danwinship danwinship deleted the better-fuzzing branch May 30, 2017 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants