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

Add python tests as a new prow job. #1360

Merged
merged 1 commit into from
Dec 14, 2016
Merged

Add python tests as a new prow job. #1360

merged 1 commit into from
Dec 14, 2016

Conversation

spxtr
Copy link
Contributor

@spxtr spxtr commented Dec 13, 2016

#1342. We have two options here. We can either

  1. put all of our tests in one job, or
  2. try to find a logical grouping of tests and have several jobs, or
  3. use bazel for everything.

Both are pretty good. In this PR I add pull-test-infra-py-test, as a step down the second path. There is some duplication in /images/, and I'd like to clean that up in a future PR if we go down this route. I'm partial to bazel, but that's a bit trickier.

@spxtr spxtr assigned rmmh, fejta and ixdy Dec 13, 2016
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 13, 2016
@k8s-reviewable
Copy link

This change is Reviewable

# See the License for the specific language governing permissions and
# limitations under the License.

VERSION = 0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer we use the same version system as with kubekins-e2e

@fejta
Copy link
Contributor

fejta commented Dec 13, 2016

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2016
- name: pull-test-infra-py-test
always_run: true
skip_report: true
context: py test
Copy link
Member

Choose a reason for hiding this comment

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

should we call this "prow py test"? or rename the other one from "prow go test" to just "go test?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should rename the other to just "go test"

@ixdy
Copy link
Member

ixdy commented Dec 14, 2016

I think I'd prefer having fewer jobs to worry about.

@ixdy
Copy link
Member

ixdy commented Dec 14, 2016

Also the levels of indirection are starting to hurt my brain:

  1. prow runs the test-infra-py-test container
  2. the container runner checks out the test-infra master branch and runs bootstrap
  3. bootstrap checks out test-infra at the desired merge commit and runs the job script
  4. the job script actually finally runs the tests we care about (more python)

I guess the first three steps can be generalized/parameterized, which would reduce some of the cognitive overhead. I can't imagine having to set up all of these steps every time we want to start a new job.

@spxtr
Copy link
Contributor Author

spxtr commented Dec 14, 2016

Yeah, it does feel a bit complicated. We can cut that back by moving bootstrap functionality into prow itself, if that makes sense.

@spxtr spxtr merged commit 12eab9c into kubernetes:master Dec 14, 2016
@spxtr spxtr deleted the pytest branch December 14, 2016 21:39
foxish pushed a commit to foxish/test-infra that referenced this pull request Jan 21, 2017
Remove gce-scalability from blockers list
ostromart pushed a commit to ostromart/test-infra that referenced this pull request Jul 26, 2019
grantr pushed a commit to grantr/test-infra that referenced this pull request Feb 21, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants