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

Tests adaptation for new header name format in HTTP tables directives. #31

Merged
merged 2 commits into from
Jul 18, 2018

Conversation

aleksostapenko
Copy link
Contributor

No description provided.

@vankoven
Copy link
Contributor

@vladtcvs this PR requires tempesta repo on branch ao-1033. Tests will fail if tempesta is checkecked to any other branch. In the same time tests for branch ao-1033 will fail if the tests doesn't involve changes from this PR. This will cause CI check failure for tempesta-tech/tempesta#1036. That's not good.

Is there any way to solve the issue?

Copy link
Contributor

@vankoven vankoven left a comment

Choose a reason for hiding this comment

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

Good but one test cese is missing, need to add it before the merge

' hdr Host == "*natsys-lab.com" -> hdr_h_s;\n'
' hdr Referer == "example.com" -> hdr_r_e;\n'
' hdr Referer == "*.com" -> hdr_r_s;\n'
' hdr referer == "http://example.com*" -> hdr_r_p;\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add rule for raw header comparison. That branch of code is not tested here or anywhere eslse if i'm not mstaken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@vladtcvs
Copy link
Contributor

Is there any way to solve the issue?

Yes, you should run CI with required branch

@aleksostapenko aleksostapenko merged commit ca61c83 into master Jul 18, 2018
@aleksostapenko aleksostapenko deleted the ao-1033 branch July 18, 2018 19:06
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.

3 participants