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

Refactor e2e tests and update go dependencies #2573

Merged
merged 6 commits into from
May 28, 2018
Merged

Refactor e2e tests and update go dependencies #2573

merged 6 commits into from
May 28, 2018

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented May 26, 2018

No description provided.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 26, 2018
antoineco
antoineco reviewed May 26, 2018
View reviewed changes
Makefile
@@ -202,6 +202,7 @@ dep-ensure:
dep version || go get -u github.com/golang/dep/cmd/dep
dep ensure -v
dep prune -v
find vendor -name '*_test.go' -delete
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea why test files weren't pruned despite go-tests = true?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I even tried to update dep and the result was the same.

@aledbf aledbf added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 26, 2018
@aledbf
Copy link
Member Author

aledbf commented May 26, 2018

The new makefile task is required to avoid issues when we build the nginx image or the ingress controller
This is the script used for that https://gist.github.com/aledbf/e897c01f9fa285fccac04f8fd0e1047a

@aledbf aledbf changed the title Update go dependencies Refactor e2e tests and update go dependencies May 26, 2018
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 26, 2018
@codecov-io
Copy link

Codecov Report

Merging #2573 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2573   +/-   ##
=======================================
  Coverage   40.63%   40.63%           
=======================================
  Files          74       74           
  Lines        5094     5094           
=======================================
  Hits         2070     2070           
  Misses       2743     2743           
  Partials      281      281

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15ffb51...2c226ef. Read the comment docs.

ElvinEfendi
ElvinEfendi reviewed May 28, 2018
View reviewed changes
Makefile
@@ -109,8 +115,6 @@ ifeq ($(ARCH),amd64)
$(SED_I) "/CROSS_BUILD_/d" $(DOCKERFILE)
else
# When cross-building, only the placeholder "CROSS_BUILD_" should be removed
# Register /usr/bin/qemu-ARCH-static as the handler for ARM binaries in the kernel
$(DOCKER) run --rm --privileged multiarch/qemu-user-static:register --reset
Copy link
Member

@ElvinEfendi ElvinEfendi May 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

So we don't need to do this in CI? I don't see any reference to make register-qemu in .travis.yml

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we don't use this in CI (it takes too long to run, that is why I build docker images using a VM)

@antoineco
Copy link
Contributor

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, antoineco

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 merged commit c34aeed into kubernetes:master May 28, 2018
@aledbf aledbf deleted the update-go-deps branch May 28, 2018 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoineco antoineco antoineco left review comments

@ElvinEfendi ElvinEfendi ElvinEfendi left review comments

Assignees

@ElvinEfendi ElvinEfendi

@antoineco antoineco

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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aledbf @codecov-io @antoineco @k8s-ci-robot @ElvinEfendi