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

added test for map variables #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

amgorb
Copy link

@amgorb amgorb commented Dec 30, 2016

No description provided.

@@ -647,3 +647,30 @@ GET /test
nil
--- no_error_log
[error]

=== TEST 19: upstream_name with valid explicit upstream set by variable
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of this test case? Testing the standard map directive?

Copy link
Author

Choose a reason for hiding this comment

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

Point is to test situation where current_upstream_name() is called with block of upstream set by variable. For example, current_upstream_name() works fine in issue
#39 when scheme is explicitly set (location: /test). And returns error when scheme is set via variable (location: /test2).

@agentzh
Copy link
Member

agentzh commented Jan 24, 2017

@amgorb The Travis CI testing is failing. Are you reporting a bug or just enriching the test suite? Please always be explicit.

@amgorb
Copy link
Author

amgorb commented Feb 2, 2017

@agentzh Yep, tests are failing because this bug isnt fixed yep: https://travis-ci.org/openresty/lua-upstream-nginx-module/jobs/187700844.

To clarify more:
this issue #39 descibed a bug in upstream.current_upstream_name which fails to display upstream name if scheme is set via variable

The test case in this pullreq will pass when issue will be fixed. Cant find the exact problem location

@agentzh
Copy link
Member

agentzh commented Feb 2, 2017

@amgorb I cannot merge a PR with failing tests to break our workflow. A corresponding fix is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants