Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

chore(ingress): Adding unit tests - #1283

Merged
krisnova merged 2 commits into
deis:masterfrom
krisnova:ingress-unit-tests
Apr 12, 2017
Merged

chore(ingress): Adding unit tests#1283
krisnova merged 2 commits into
deis:masterfrom
krisnova:ingress-unit-tests

Conversation

@krisnova

@krisnova krisnova commented Apr 11, 2017 •
edited by bacongobbler
Loading

Copy link
Copy Markdown
Contributor

Adding unit tests for ingress

Non breaking change

closes #1275
@deis-bot

deis-bot commented Apr 11, 2017

Copy link
Copy Markdown

@bacongobbler, @mboersma and @rvadim are potential reviewers of this pull request based on my analysis of git blame information. Thanks @kris-nova!
@krisnova
chore(ingress): Adding unit tests
49e741e 
Adding unit tests for ingress

Non breaking change
@bacongobbler

bacongobbler commented Apr 11, 2017

Copy link
Copy Markdown
Member

looks like make test-style is failing on this PR. Would you mind fixing that up? Thanks!
@krisnova
fix(ingress-test): Fixing make test-style
960e899 
Fixing some formatting concerns in make test-style

Non breaking change
@krisnova

krisnova commented Apr 11, 2017

Copy link
Copy Markdown
Contributor Author

Oh dang, thanks @bacongobbler they should be fixed now.
@mboersma mboersma added this to the v2.14 milestone Apr 11, 2017
@codecov-io

codecov-io commented Apr 11, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1283 into master will increase coverage by 0.38%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
+ Coverage   86.66%   87.04%   +0.38%     
==========================================
  Files          45       45              
  Lines        3929     3929              
  Branches      681      681              
==========================================
+ Hits         3405     3420      +15     
+ Misses        355      338      -17     
- Partials      169      171       +2

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 092e8fe...960e899. Read the comment docs.
@mboersma mboersma added the LGTM1 label Apr 11, 2017
bacongobbler
bacongobbler approved these changes Apr 12, 2017
View reviewed changes
Comment thread rootfs/scheduler/resources/ingress.py

def delete(self, namespace, ingress):
url = self.api("/namespaces/{}/ingresses/{}", namespace, ingress)
url = "/apis/extensions/v1beta1/namespaces/%s/ingresses/%s" % (namespace, ingress)

@bacongobbler bacongobbler Apr 12, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good catch. I also just checked to confirm and this API endpoint is also the same for v1.5. We'll have to refactor the self.api function now that kubernetes is going with the aggregated API route, but this works for now. 👍

I'll file an issue for this.
@bacongobbler bacongobbler mentioned this pull request Apr 12, 2017
Open
@krisnova
krisnova merged commit 465c580 into deis:master Apr 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

LGTM1 LGTM2

5 participants

@krisnova @deis-bot @bacongobbler @codecov-io @mboersma