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

http integration tests: Migrate c_str to getStringView #87

Closed
dio opened this issue Jun 1, 2019 · 2 comments · Fixed by #88
Closed

http integration tests: Migrate c_str to getStringView #87

dio opened this issue Jun 1, 2019 · 2 comments · Fixed by #88
Labels
good first issue Good first issue help wanted

Comments

@dio
Copy link
Member

dio commented Jun 1, 2019 •
edited
Loading

As reported in: envoyproxy/envoy#7082 (comment).

We can fix the issue by modifying https://github.com/envoyproxy/envoy-filter-example/blob/master/http-filter-example/http_filter_integration_test.cc#L39. And this PR envoyproxy/envoy#6564 for inspiration.

cc. @Satvindar
@dio dio added good first issue Good first issue help wanted labels Jun 1, 2019
@Satvindar
Copy link
Contributor

@dio, I have fixed this issue and build is successful now :)
But when I am trying to create a PR, I am getting access denied error. I am sure I am missing some configuration before I push my changes to envoy git. Can you help with the steps to follow for pushing my changes to envoy git? Thanks.

remote: Permission to envoyproxy/envoy-filter-example.git denied to Satvindar.
fatal: unable to access 'https://github.com/envoyproxy/envoy-filter-example.git/': The requested URL returned error: 403

@dio
Copy link
Member Author

dio commented Jun 3, 2019

@Satvindar you need to work on your own fork. Please see e.g. this: https://gist.github.com/Chaser324/ce0505fbed06b947d962

@Satvindar Satvindar mentioned this issue Jun 4, 2019
Merged
@dio dio closed this as completed in #88 Jun 4, 2019
dio pushed a commit that referenced this issue Jun 4, 2019
@Satvindar @dio
test: Fix http integration test
c88e0c4 
Replace the `HeaderString::c_str()` API with `getStringView()` and `EXPECT_STREQ` with `EXPECT_EQ` for string objects comparison.

Docs Changes: N/A
Release Notes: N/A
Fixes #87

Signed-off-by: Satvindar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good first issue help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: Fix http integration test Satvindar/envoy-filter-example
2 participants
@dio @Satvindar