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

Promote non-table based container-runtime e2e test to Conformance #67612

Merged
merged 1 commit into from Sep 25, 2018

Conversation

@mgdevstack
Member

mgdevstack commented Aug 20, 2018

What this PR does / why we need it:
This PR is Promoting container-runtime e2e tests to conformance. Few of container-runtime e2e_node tests are ported into e2e in #67571.

Promoting only non-table based e2e test-

Container Runtime blackbox test when starting a container that exits should run with the expected status [NodeConformance]

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

Special notes for your reviewer:
Few challenges are metioned in #67609 .

  1. Does it make sense to update structs to incorporate conformance comment block and is it useful?
  2. Nested It() statements/e2e tests are not populated in golden conformance list. Only mentioned test#1 is added into the golden list.

Release note:

NONE

/area conformance
@kubernetes/sig-node-pr-reviews

@k8s-ci-robot k8s-ci-robot requested review from derekwaynecarr and rmmh Aug 20, 2018

@mgdevstack mgdevstack changed the title from [WIP] Promoting container-runtime e2e tests to Conformance Suite to Promote container-runtime e2e tests to Conformance Suite Sep 3, 2018

@mgdevstack

This comment has been minimized.

Show comment
Hide comment
@mgdevstack

mgdevstack Sep 3, 2018

Member

/hold

Member

mgdevstack commented Sep 3, 2018

/hold

@feiskyer

This comment has been minimized.

Show comment
Hide comment
@feiskyer

feiskyer Sep 3, 2018

Member

/retest

Member

feiskyer commented Sep 3, 2018

/retest

@brahmaroutu

This comment has been minimized.

Show comment
Hide comment
@brahmaroutu

brahmaroutu Sep 4, 2018

Contributor

@mgdevstack It is not possible to generate Conformance doc at runtime. We use ast scanner to pick up comment block statically, even the fmt statement in the ConformanceIt will be a issue when generating the doc, only literals are used. For a table of tests, please use the large comment block with a format like
//What is tested by this group of tests, followed by each test description
// test1: Test a external image ...

Comment struct does not take affect with the approach, please refactor accordingly.

Contributor

brahmaroutu commented Sep 4, 2018

@mgdevstack It is not possible to generate Conformance doc at runtime. We use ast scanner to pick up comment block statically, even the fmt statement in the ConformanceIt will be a issue when generating the doc, only literals are used. For a table of tests, please use the large comment block with a format like
//What is tested by this group of tests, followed by each test description
// test1: Test a external image ...

Comment struct does not take affect with the approach, please refactor accordingly.

@mgdevstack

This comment has been minimized.

Show comment
Hide comment
@mgdevstack

mgdevstack Sep 5, 2018

Member

fmt statement in the ConformanceIt will be a issue when generating the doc, only literals are used.

@brahmaroutu , this is the reason table based tests are not getting updated into Conformance golden list because of fmt.Sprintf() leading to error -

ERROR at test/e2e/common/runtime.go:235:29: framework.ConformanceIt() called with non-literal argument

What would be your suggesion to encounter this issue?

Member

mgdevstack commented Sep 5, 2018

fmt statement in the ConformanceIt will be a issue when generating the doc, only literals are used.

@brahmaroutu , this is the reason table based tests are not getting updated into Conformance golden list because of fmt.Sprintf() leading to error -

ERROR at test/e2e/common/runtime.go:235:29: framework.ConformanceIt() called with non-literal argument

What would be your suggesion to encounter this issue?

@mgdevstack

This comment has been minimized.

Show comment
Hide comment
@mgdevstack

mgdevstack Sep 20, 2018

Member

/hold cancel

Member

mgdevstack commented Sep 20, 2018

/hold cancel

@mgdevstack mgdevstack changed the title from Promote container-runtime e2e tests to Conformance Suite to Promote non-table based container-runtime e2e test to Conformance Sep 20, 2018

@spiffxp

This comment has been minimized.

Show comment
Hide comment
@spiffxp

spiffxp Sep 21, 2018

Member

/milestone v1.13
/lgtm
/assign @bgrant0607 @smarterclayton
for /approve

Member

spiffxp commented Sep 21, 2018

/milestone v1.13
/lgtm
/assign @bgrant0607 @smarterclayton
for /approve

@spiffxp

This comment has been minimized.

Show comment
Hide comment
@spiffxp

spiffxp Sep 21, 2018

Member

This is one approach of working around a table based test: wrapping the whole table in an It statement.

Member

spiffxp commented Sep 21, 2018

This is one approach of working around a table based test: wrapping the whole table in an It statement.

@smarterclayton

This comment has been minimized.

Show comment
Hide comment
@smarterclayton

smarterclayton Sep 22, 2018

Contributor

/approve

Contributor

smarterclayton commented Sep 22, 2018

/approve

@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Sep 22, 2018

Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgdevstack, smarterclayton, spiffxp

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Contributor

k8s-ci-robot commented Sep 22, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mgdevstack, smarterclayton, spiffxp

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spiffxp

This comment has been minimized.

Show comment
Hide comment
@spiffxp

spiffxp Sep 24, 2018

Member

/retest

Member

spiffxp commented Sep 24, 2018

/retest

@spiffxp

This comment has been minimized.

Show comment
Hide comment
@spiffxp

spiffxp Sep 24, 2018

Member

/retest

Member

spiffxp commented Sep 24, 2018

/retest

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Sep 25, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

fejta-bot commented Sep 25, 2018

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit c00f45c into kubernetes:master Sep 25, 2018

18 checks passed

cla/linuxfoundation mgdevstack authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment