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

key-auth+cli e2e test connect #345 #377

Merged
merged 4 commits into from
Aug 30, 2017
Merged

key-auth+cli e2e test connect #345 #377

merged 4 commits into from
Aug 30, 2017

Conversation

DrMegavolt
Copy link
Contributor

@DrMegavolt DrMegavolt commented Aug 14, 2017 •
edited
Loading

connect #345

@DrMegavolt DrMegavolt self-assigned this Aug 14, 2017
@DrMegavolt DrMegavolt requested a review from kevinswiber August 14, 2017 15:29
@codecov
Copy link

codecov bot commented Aug 14, 2017 •
edited
Loading

Codecov Report

Merging #377 into master will increase coverage by 0.52%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #377      +/-   ##
==========================================
+ Coverage   90.93%   91.46%   +0.52%     
==========================================
  Files         193      197       +4     
  Lines        8214     8349     +135     
==========================================
+ Hits         7469     7636     +167     
+ Misses        745      713      -32
Impacted Files Coverage Δ
lib/policies/basic-auth/auth.js 91.66% <ø> (ø) ⬆️
lib/services/auth.js 91.42% <ø> (ø) ⬆️
test/policies/basic-auth-policy.test.js 96.55% <100%> (+0.96%) ⬆️
test/e2e/basic-auth.e2e.test.js 100% <100%> (ø)
test/e2e/key-auth.e2e.test.js 100% <100%> (ø)
...olicy-seq-oauth2-expression-log-ratelimit-proxy.js 97.75% <100%> (ø) ⬆️
test/e2e/oauth2-authorization-code.js 99.13% <100%> (+3.78%) ⬆️
test/e2e/round-robin.test.js 91.78% <50%> (-0.12%) ⬇️
test/e2e/hot-reload.test.js 89.53% <80%> (ø)
test/common/gateway.helper.js 86.36% <86.36%> (ø)
... and 10 more

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 f9ac285...3992caa. Read the comment docs.

@altsang altsang added test and removed in progress labels Aug 14, 2017
kevinswiber
kevinswiber reviewed Aug 14, 2017
View reviewed changes
test/common/gateway.helper.js Outdated
module.exports.startGatewayInstance = function ({dirInfo, gatewayConfig}) {
return this.findOpenPortNumbers(4)
.then(ports => {
console.log(ports);
Copy link
Member

Choose a reason for hiding this comment

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

Should be able to get rid of the console.log statement.

kevinswiber
kevinswiber reviewed Aug 15, 2017
View reviewed changes
test/common/gateway.helper.js
reject(err);
});
let count = 0;
let interval = setInterval(() => {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@kevinswiber
Copy link
Member

@DrMegavolt Can you refactor the OAuth2 Authorization Code test to take advantage of the new helpers you built and perhaps throw in the retry counter you added?

@DrMegavolt DrMegavolt changed the title key-auth+cli e2e test key-auth+cli e2e test connect #345 Aug 15, 2017
@DrMegavolt
Copy link
Contributor Author

@kevinswiber auth-code and hot-reload now uses refactored find ports method.
Create Gateway is bit different in each case, probably my helper will need revision once we have more use cases, for now I left original code

@DrMegavolt DrMegavolt force-pushed the feature/e2e/key-auth branch from cdaab0b to b72ecab Compare August 15, 2017 19:03
@DrMegavolt DrMegavolt force-pushed the feature/e2e/key-auth branch from ce341fc to 3992caa Compare August 16, 2017 10:07
@kevinswiber kevinswiber merged commit 41a14f5 into master Aug 30, 2017
@kevinswiber kevinswiber deleted the feature/e2e/key-auth branch August 30, 2017 23:08
@kevinswiber kevinswiber removed the test label Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinswiber kevinswiber kevinswiber approved these changes

Assignees

@DrMegavolt DrMegavolt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DrMegavolt @kevinswiber @altsang