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

internal/featuretests: add simple HTTPProxy fixtures #2519

Merged
merged 1 commit into from
May 20, 2020

Conversation

jpeach
Copy link
Contributor

@jpeach jpeach commented May 12, 2020

Add some simple helpers to generate HTTPProxy test fixtures. This
helps reduce some boilerplate in setting up test cases.

Signed-off-by: James Peach jpeach@vmware.com

@jpeach jpeach requested review from stevesloka and youngnick May 12, 2020 07:26
@jpeach
Copy link
Contributor Author

jpeach commented May 12, 2020

@stevesloka @youngnick Let me know what you think of this approach. We can remove a bit more boilerplate by adding more fluent helpers and extending to other types.

@codecov
Copy link

codecov bot commented May 12, 2020

Codecov Report

Merging #2519 into master will increase coverage by 0.08%.
The diff coverage is 91.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2519      +/-   ##
==========================================
+ Coverage   76.97%   77.05%   +0.08%     
==========================================
  Files          68       70       +2     
  Lines        5707     5732      +25     
==========================================
+ Hits         4393     4417      +24     
- Misses       1218     1219       +1     
  Partials       96       96              
Impacted Files Coverage Δ
internal/featuretests/kubernetes.go 100.00% <ø> (+11.76%) ⬆️
internal/fixture/httpproxy.go 86.95% <86.95%> (ø)
internal/fixture/meta.go 100.00% <100.00%> (ø)

Copy link
Member

@stevesloka stevesloka left a comment

Choose a reason for hiding this comment

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

I think this is fine. We might also need a .WithNamespace() or something to allow different namespaces, but obviously can come when needed.

@jpeach
Copy link
Contributor Author

jpeach commented May 19, 2020

I think this is fine. We might also need a .WithNamespace() or something to allow different namespaces, but obviously can come when needed.

.WithName accepts namespace/name syntax to set the namespace, but you are right, we can add a new namespace setter too if it's convenient.

Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

LGTM, I really like this, it will let us remove the package-specific fixtures.

@jpeach jpeach force-pushed the wip-proxy-fixtures branch from cf93c75 to 145d623 Compare May 19, 2020 06:21
@jpeach jpeach marked this pull request as ready for review May 19, 2020 06:21
@jpeach
Copy link
Contributor Author

jpeach commented May 19, 2020

FWIW the Travis checks passed but the GitHub status wasn't updated.

Add some simple helpers to generate HTTPProxy test fixtures. This
helps reduce some boilerplate in setting up test cases.

Signed-off-by: James Peach <jpeach@vmware.com>
@jpeach jpeach force-pushed the wip-proxy-fixtures branch from 145d623 to 2d14850 Compare May 19, 2020 23:51
@jpeach jpeach merged commit bec00c2 into projectcontour:master May 20, 2020
@jpeach jpeach deleted the wip-proxy-fixtures branch May 20, 2020 00:24
@jpeach jpeach added this to the 1.5.0 milestone May 28, 2020
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