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

Do not assert against Generation in conformance tests #475

Merged
merged 1 commit into from
Mar 23, 2018

Conversation

bobcatfish
Copy link
Contributor

The values the conformance test was asserting against were wrong.
Route.Generation and Configuration.Generation should actually have
a value of 1 when they are first created, and the *.Status.Generation
values are just hacks until CRD generations are updated properly
(which is fixed in kubernetes/kubernetes#58778
but we are not yet using).

I want to remove these assertions since they are just asserting
against a hack. We should actually assert against
Configuration.Generation and Route.Generation when we have pulled in the
fix.

The values the conformance test was asserting against were wrong.
Route.Generation and Configuration.Generation should actually have
a value of 1 when they are first created, and the *.Status.Generation
values are just hacks until CRD generations are updated properly
(which is fixed in kubernetes/kubernetes#58778
but we are not yet using).

I want to remove these assertions since they are just asserting
against a hack. We should actually assert against
Configuration.Generation and Route.Generation when we have pulled in the
fix.
@google-prow-robot google-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 22, 2018
@google-prow-robot
Copy link

google-prow-robot commented Mar 22, 2018

@bobcatfish: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-elafros-go-coverage-test 4a3c1c5 link /test pull-elafros-go-coverage-test

Full PR test history. Your PR dashboard.

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.

@adrcunha
Copy link
Contributor

/approve

@google-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrcunha, bobcatfish

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

@bobcatfish bobcatfish merged commit 13b7b1e into knative:master Mar 23, 2018
@bobcatfish bobcatfish deleted the remove_conformance_generation branch March 23, 2018 22:07
google-prow-robot pushed a commit that referenced this pull request Jul 20, 2018
…1428)

In #475 I removed assertions
against Configuration.Spec.Generation b/c it is a hack and not part of
the knative spec.

In #600 I updated the
conformance tests to assert against the Revision annotation which
contains the generation.

BUT THEN in #778 when I
completely re-wrote the tests to no longer use Ginkgo, I accidentally
undid both of those changes, so this commit puts them back 😅.

BONUS: I hit a case where the length of the loadbalancer ingresses was
0 and got a panic, so if that happens again we'll get an informative
error instead.
nak3 added a commit to nak3/serving that referenced this pull request Jun 8, 2020
* Update catalogsource to 1.8.0

This patch updates catalogsource to v1.8.0.

Also, it updates `openshift/olm/catalog.sh` to use minimal
catalogsource. Previously the catalogsource contains all version, but
it cuts unnecessary old versions for CI.

* Add comment for patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants