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.

Sign up for GitHub

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

Jump to bottom

Unbreak the E2E tests #370

Merged
merged 4 commits into from
Jan 9, 2019
Merged

Unbreak the E2E tests #370

merged 4 commits into from
Jan 9, 2019

Conversation

adrcunha
Copy link
Contributor

@adrcunha adrcunha commented Jan 8, 2019 •
edited
Loading

Fixes #361.
Fixes #368.

Update test-infra

Major changes:

  • Also update E2E test return value on failures
  • Default build/unit test runners

Fix botched rename of PipelineTrigger field

This is a copy of #369.

Disable broken tests

There are 2 tests failing consistently, and it seems that they're failing only on Prow. Disabling them here to unblock the E2E tests, filed #375 and #376 to track the fixes.

@adrcunha
Update test-infra
1079efb 
Major changes:
* Also update E2E test return value on failures
* Default build/unit test runners
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 8, 2019
@adrcunha adrcunha mentioned this pull request Jan 8, 2019
Closed
imjasonh
imjasonh requested changes Jan 8, 2019
View reviewed changes
test/presubmit-tests.sh Show resolved Hide resolved
imjasonh
imjasonh approved these changes Jan 9, 2019
View reviewed changes
Copy link
Member

@imjasonh imjasonh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 9, 2019
@bobcatfish
Copy link
Collaborator

bobcatfish commented Jan 9, 2019 •
edited
Loading

@adrcunha i think you might need to bring in the changes from #361 #369 (whoops wrong link) since the integration tests are broken at head before this will merge 😭

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Apply the fixes from #369
b9e7c90 
Otherwise E2E tests won't pass.
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2019
@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

6 similar comments
@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/retest

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

Nevermind the retests, this is not flakiness:

I0109 15:14:22.583]     --- FAIL: TestPipelineRun/fan-in_and_fan-out (600.37s)
I0109 15:14:22.583]         pipelinerun_test.go:153: Error waiting for PipelineRun helloworld-pipelinerun1 to finish: timed out waiting for the condition
...
I0109 15:16:14.195] error: error validating "STDIN": error validating data: ValidationError(ClusterRoleBinding): unknown field "spec" in io.k8s.api.rbac.v1beta1.ClusterRoleBinding; if you choose to ignore these errors, turn validation off with --validate=false
I0109 15:16:14.200] 2019/01/09 15:16:14 error executing 'kubectl apply': exit status 1
I0109 15:16:14.201] ERROR: one or more YAML tests failed

@bobcatfish do you know what's wrong here? I don't think that this is a simple case of increasing the timeout (and it won't fix the yaml tests anyway)

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

As discussed on Slack, the errors in E2E seem to be related to the Prow environment. I'm gonna disable the broken tests to unblock the pipeline and file issues for them.

@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

Tests passed, let me file issues and update the PR description...

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 9, 2019
@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

Filed #375 and #376 for the broken tests.

@adrcunha adrcunha changed the title Update test-infra Unbreak the E2E tests Jan 9, 2019
@adrcunha
Copy link
Contributor Author

adrcunha commented Jan 9, 2019

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 9, 2019
@imjasonh
Copy link
Member

imjasonh commented Jan 9, 2019

/lgtm
/approve

🎉

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2019
@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@knative-prow-robot knative-prow-robot merged commit e2f4e40 into tektoncd:master Jan 9, 2019
@adrcunha adrcunha deleted the fixe2e branch January 9, 2019 19:26
@adrcunha adrcunha mentioned this pull request Jan 9, 2019
Merged
chmouel pushed a commit to chmouel/tektoncd-pipeline that referenced this pull request Apr 7, 2020
@openshift-merge-robot
Merge pull request tektoncd#370 from chmouel/remove-ko-from-release
d7ef46a 
Remove ko:// prefix from release.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imjasonh imjasonh imjasonh approved these changes

@bobcatfish bobcatfish Awaiting requested review from bobcatfish

@dushyanthsc dushyanthsc Awaiting requested review from dushyanthsc

@shashwathi shashwathi Awaiting requested review from shashwathi

Assignees

@bobcatfish bobcatfish

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

E2E tests broken at head due to broken rename in #366 Nightly releases are published even if E2E tests fail
4 participants
@adrcunha @bobcatfish @imjasonh @knative-prow-robot