Ignore unexported fields in import_known_versions_test #53402

dougm · 2017-10-03T20:02:33Z

Tests currently fail with:

"import_known_versions_test.go:122: Unexpected type uint in ..."

What this PR does / why we need it:

Running make test against the latest (f11a551) fails for me with:

import_known_versions_test.go:122: Unexpected type uint in schema.GroupVersionKind{Group:"apps", Version:"__internal", Kind:"DaemonSet"}

        import_known_versions_test.go:122: Unexpected type uint in schema.GroupVersionKind{Group:"apps", Version:"__internal", Kind:"DaemonSet"}
        import_known_versions_test.go:124: extensions.DaemonSet:
        import_known_versions_test.go:124:   extensions.DaemonSetSpec:
        import_known_versions_test.go:124:     api.PodTemplateSpec:
        import_known_versions_test.go:124:       api.PodSpec:
        import_known_versions_test.go:124:         []api.Container:
        import_known_versions_test.go:124:           api.Container:
        import_known_versions_test.go:124:             []api.EnvVar:
        import_known_versions_test.go:124:               api.EnvVar:
        import_known_versions_test.go:124:                 *api.EnvVarSource:
        import_known_versions_test.go:124:                   api.EnvVarSource:
        import_known_versions_test.go:124:                     *api.ResourceFieldSelector:
        import_known_versions_test.go:124:                       api.ResourceFieldSelector:
        import_known_versions_test.go:124:                         resource.Quantity:
        import_known_versions_test.go:124:                           resource.infDecAmount:
        import_known_versions_test.go:124:                             *inf.Dec:
        import_known_versions_test.go:124:                               inf.Dec:
        import_known_versions_test.go:124:                                 big.Int:
        import_known_versions_test.go:124:                                   big.nat:
        import_known_versions_test.go:124:                                     big.Word:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #53508

Special notes for your reviewer:

Release note:

NONE

k8s-ci-robot · 2017-10-03T20:02:41Z

Hi @dougm. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

dougm · 2017-10-03T22:59:03Z

/sig testing

thockin · 2017-10-04T19:21:40Z

/ok-to-test

thockin · 2017-10-04T19:21:56Z

This seems OK, but what changed to make this start failing?

dougm · 2017-10-04T19:46:10Z

Seems the type of an unexported field changed. The test also passes when ignoring unexported fields:

modified   pkg/master/import_known_versions_test.go
@@ -106,6 +106,10 @@ func ensureNoTags(t *testing.T, gvk schema.GroupVersionKind, tp reflect.Type, pa
 	case reflect.Struct:
 		for i := 0; i < tp.NumField(); i++ {
 			f := tp.Field(i)
+			if f.PkgPath != "" {
+				continue
+			}
+
 			jsonTag := f.Tag.Get("json")
 			protoTag := f.Tag.Get("protobuf")
 			if len(jsonTag) > 0 || len(protoTag) > 0 {

thockin · 2017-10-04T20:33:42Z

Is that the better fix? We don't really want people using `int` and `uint` in API.

…

On Wed, Oct 4, 2017 at 12:46 PM, Doug MacEachern ***@***.***> wrote: Seems the type of an unexported field changed. The test also passes when ignoring unexported fields: modified pkg/master/import_known_versions_test.go@@ -106,6 +106,10 @@ func ensureNoTags(t *testing.T, gvk schema.GroupVersionKind, tp reflect.Type, pa case reflect.Struct: for i := 0; i < tp.NumField(); i++ { f := tp.Field(i)+ if f.PkgPath == "" {+ continue+ }+ jsonTag := f.Tag.Get("json") protoTag := f.Tag.Get("protobuf") if len(jsonTag) > 0 || len(protoTag) > 0 { — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#53402 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVPxRk5mOUFrGjDUa91HzuJgL_Popks5so-CjgaJpZM4PssNF> .

dougm · 2017-10-04T22:41:13Z

@thockin sounds good, I've updated the PR to ignore unexported fields instead.

The field of type big.Word displayed in the logs above that is a reflect.Uint: https://github.com/golang/go/blob/d36cc9baf3c4127ef5c98bc8844b405d63064e48/src/math/big/arith.go#L14

dougm · 2017-10-04T22:43:41Z

Also note that I tried removing reflect.Int from the filtered types, but there are a few fields of type int. Not sure if you'd want that addressed in this PR or a separate issue?

        import_known_versions_test.go:125: Unexpected type int in schema.GroupVersionKind{Group:"extensions", Version:"__internal", Kind:"PodSecurityPolicyList"}
        import_known_versions_test.go:127: extensions.PodSecurityPolicyList:
        import_known_versions_test.go:127:   []extensions.PodSecurityPolicy:
        import_known_versions_test.go:127:     extensions.PodSecurityPolicy:
        import_known_versions_test.go:127:       extensions.PodSecurityPolicySpec:
        import_known_versions_test.go:127:         []extensions.HostPortRange:
        import_known_versions_test.go:127:           extensions.HostPortRange:
        import_known_versions_test.go:127:             int:
        import_known_versions_test.go:125: Unexpected type int in schema.GroupVersionKind{Group:"extensions", Version:"__internal", Kind:"PodSecurityPolicyList"}
        import_known_versions_test.go:127: extensions.PodSecurityPolicyList:
        import_known_versions_test.go:127:   []extensions.PodSecurityPolicy:
        import_known_versions_test.go:127:     extensions.PodSecurityPolicy:
        import_known_versions_test.go:127:       extensions.PodSecurityPolicySpec:
        import_known_versions_test.go:127:         []extensions.HostPortRange:
        import_known_versions_test.go:127:           extensions.HostPortRange:
        import_known_versions_test.go:127:             int:
        import_known_versions_test.go:125: Unexpected type int in schema.GroupVersionKind{Group:"componentconfig", Version:"__internal", Kind:"KubeSchedulerConfiguration"}
        import_known_versions_test.go:127: componentconfig.KubeSchedulerConfiguration:
        import_known_versions_test.go:127:   int:
        import_known_versions_test.go:125: Unexpected type int in schema.GroupVersionKind{Group:"componentconfig", Version:"__internal", Kind:"KubeProxyConfiguration"}
        import_known_versions_test.go:127: componentconfig.KubeProxyConfiguration:
        import_known_versions_test.go:127:   componentconfig.ClientConnectionConfiguration:
        import_known_versions_test.go:127:     int:
        import_known_versions_test.go:125: Unexpected type int in schema.GroupVersionKind{Group:"extensions", Version:"__internal", Kind:"PodSecurityPolicy"}
        import_known_versions_test.go:127: extensions.PodSecurityPolicy:
        import_known_versions_test.go:127:   extensions.PodSecurityPolicySpec:
        import_known_versions_test.go:127:     []extensions.HostPortRange:
        import_known_versions_test.go:127:       extensions.HostPortRange:
        import_known_versions_test.go:127:         int:
        import_known_versions_test.go:125: Unexpected type int in schema.GroupVersionKind{Group:"extensions", Version:"__internal", Kind:"PodSecurityPolicy"}
        import_known_versions_test.go:127: extensions.PodSecurityPolicy:
        import_known_versions_test.go:127:   extensions.PodSecurityPolicySpec:
        import_known_versions_test.go:127:     []extensions.HostPortRange:
        import_known_versions_test.go:127:       extensions.HostPortRange:
        import_known_versions_test.go:127:         int:

dougm · 2017-10-05T02:39:07Z

/retest

thockin · 2017-10-05T20:30:58Z

This needs an associated issue and a closes #12345 in the FIRST COMMENT, or it can not be approved.

/lgtm

Tests currently fail with: "import_known_versions_test.go:122: Unexpected type uint in ..." Closes kubernetes#53508

thockin · 2017-10-05T21:35:47Z

ping me back when issue is filed and linked in first comment.

dougm · 2017-10-05T21:36:14Z

@thockin I created issue #53508 and added "Closes #53508" to the commit message, wasn't sure where you meant by 'FIRST COMMENT' ?

thockin · 2017-10-05T21:38:43Z

It needs to be in the first comment of this stream. Will add it.

thockin · 2017-10-05T21:39:16Z

/lgtm
/approve

k8s-github-robot · 2017-10-05T21:39:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dougm, thockin

Associated issue: 53508

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~pkg/master/OWNERS~~ [thockin]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-10-06T00:31:23Z

Automatic merge from submit-queue (batch tested with PRs 53418, 53366, 53115, 53402, 53130). If you want to cherry-pick this change to another branch, please follow the instructions here.

Changes 'int' to 'int32', enforced by import_known_versions_test Follow up to PR kubernetes#53402

Automatic merge from submit-queue (batch tested with PRs 54005, 55127, 53850, 55486, 53440). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Enforce use of fixed size int types in the API Changes 'int' to 'int32', enforced by import_known_versions_test Follow up to PR #53402 **What this PR does / why we need it**: This PR changes a few fields within the API from 'int' to 'int32' and is now enforced by import_known_versions_test. We need this so integer fields are the same size regardless of $GOARCH. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**: ```release-note ```

k8s-github-robot assigned thockin and davidopp Oct 3, 2017

k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Oct 3, 2017

thockin added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 4, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 4, 2017

dougm force-pushed the import-known-versions-test branch from ab79bf6 to 5321054 Compare October 4, 2017 22:36

dougm changed the title ~~Add uint to type filter in import_known_versions_test~~ Ignore unexported fields in import_known_versions_test Oct 4, 2017

k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 4, 2017

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 4, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2017

Ignore unexported fields in import_known_versions_test

0620569

Tests currently fail with: "import_known_versions_test.go:122: Unexpected type uint in ..." Closes kubernetes#53508

dougm force-pushed the import-known-versions-test branch from 5321054 to 0620569 Compare October 5, 2017 21:21

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2017

dougm mentioned this pull request Oct 5, 2017

import_known_versions_test fails due to unexported field of type unit #53508

Closed

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2017

k8s-github-robot merged commit feb92e1 into kubernetes:master Oct 6, 2017

dougm mentioned this pull request Oct 13, 2017

Enforce use of fixed size int types in the API #53850

Merged

dougm added a commit to dougm/kubernetes that referenced this pull request Nov 13, 2017

Enforce use of fixed size int types in the API

7950609

Changes 'int' to 'int32', enforced by import_known_versions_test Follow up to PR kubernetes#53402

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ignore unexported fields in import_known_versions_test #53402

Ignore unexported fields in import_known_versions_test #53402

dougm commented Oct 3, 2017 •

edited by thockin

k8s-ci-robot commented Oct 3, 2017

dougm commented Oct 3, 2017

thockin commented Oct 4, 2017

thockin commented Oct 4, 2017

dougm commented Oct 4, 2017 •

edited

thockin commented Oct 4, 2017 via email

dougm commented Oct 4, 2017

dougm commented Oct 4, 2017

dougm commented Oct 5, 2017

thockin commented Oct 5, 2017

thockin commented Oct 5, 2017

dougm commented Oct 5, 2017

thockin commented Oct 5, 2017

thockin commented Oct 5, 2017

k8s-github-robot commented Oct 5, 2017

k8s-github-robot commented Oct 6, 2017

Ignore unexported fields in import_known_versions_test #53402

Ignore unexported fields in import_known_versions_test #53402

Conversation

dougm commented Oct 3, 2017 • edited by thockin

k8s-ci-robot commented Oct 3, 2017

dougm commented Oct 3, 2017

thockin commented Oct 4, 2017

thockin commented Oct 4, 2017

dougm commented Oct 4, 2017 • edited

thockin commented Oct 4, 2017 via email

dougm commented Oct 4, 2017

dougm commented Oct 4, 2017

dougm commented Oct 5, 2017

thockin commented Oct 5, 2017

thockin commented Oct 5, 2017

dougm commented Oct 5, 2017

thockin commented Oct 5, 2017

thockin commented Oct 5, 2017

k8s-github-robot commented Oct 5, 2017

k8s-github-robot commented Oct 6, 2017

dougm commented Oct 3, 2017 •

edited by thockin

dougm commented Oct 4, 2017 •

edited