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

🏃 Add script to run tests in Prow #581

Merged
merged 1 commit into from
Sep 9, 2019
Merged

🏃 Add script to run tests in Prow #581

merged 1 commit into from
Sep 9, 2019

Conversation

alvaroaleman
Copy link
Member

This PR adds a script to run tests in Prow. Done to resolve #276

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 1, 2019
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 1, 2019
@alvaroaleman alvaroaleman force-pushed the prow-tests branch 3 times, most recently from 0d4e48a to aecc751 Compare September 1, 2019 19:23
@alvaroaleman alvaroaleman mentioned this pull request Sep 1, 2019
Merged
@alvaroaleman
Copy link
Member Author

Requires kubernetes/test-infra#14162 to merge, so we can verify this works. I tested this on a different Prow instance so I am reasonably confident there wont be issues.

Should I already remove the .travis.yaml or do we want to merge this -> Make Presubmit mandatory -> Remove .travis.yaml?

/assign @DirectXMan12

@DirectXMan12
Copy link
Contributor

yeah, let's get mandatory presubmit merged before we remove travis. Better to run tests twice than not at all.

DirectXMan12
DirectXMan12 suggested changes Sep 3, 2019
View reviewed changes
Copy link
Contributor

@DirectXMan12 DirectXMan12 left a comment

Choose a reason for hiding this comment

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

minor comment, otherwise lgtm

hack/ci-check-everything.sh Outdated
(
# Doesn't work with gomodules turned on, see https://github.com/golang/dep/issues/2055
export GO111MODULE=off
go get -u github.com/golang/dep/cmd/dep
Copy link
Contributor

Choose a reason for hiding this comment

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

should we just be downloading the compiled binary here? Prob way quicker.

@alvaroaleman
Copy link
Member Author

@DirectXMan12 updated the script to install a binary version of dep instead of using go get.

Could you also have a look on the pull in test-infra? It adds the new presubmit as optional, so it is safe to merge already. Once it is merged, we can verify here if everything works :)

@DirectXMan12
Copy link
Contributor

yep, looked at it. Looks like @droot isn't in the kubernetes org, but otherwise LGTM

@DirectXMan12
Copy link
Contributor

hold off on approving this till that merges?

@alvaroaleman
Copy link
Member Author

/test pull-controller-runtime-test

@alvaroaleman
Copy link
Member Author

/test pull-controller-runtime-test

1 similar comment
@alvaroaleman
Copy link
Member Author

/test pull-controller-runtime-test

@alvaroaleman alvaroaleman mentioned this pull request Sep 3, 2019
Merged
@alvaroaleman
Copy link
Member Author

Okay, so the presubmit passed, with a timeout on the first attempt. My best guess as why that happens is CPU starvation, which is why I opened kubernetes/test-infra#14184 to make the job request CPU.

IMHO this is good to go, PTAL

@DirectXMan12
Copy link
Contributor

/lgtm
/approve

awesome

excited for this

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 9, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, DirectXMan12

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2019
@k8s-ci-robot k8s-ci-robot merged commit 74c1736 into kubernetes-sigs:master Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DirectXMan12 DirectXMan12 DirectXMan12 requested changes

@droot droot Awaiting requested review from droot

@pwittrock pwittrock Awaiting requested review from pwittrock

Assignees

@DirectXMan12 DirectXMan12

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Run tests via prow
3 participants
@alvaroaleman @DirectXMan12 @k8s-ci-robot