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

[RFC] Make Service generic #1

Closed
wants to merge 11 commits into from

Conversation

hawkw
Copy link
Owner

@hawkw hawkw commented Oct 9, 2018

As described in tower-rs/tower#99, this tracks the prototype tower-rs change (hawkw/tower#1) that makes Service::Request into a type parameter, rather than an associated type.

This PR is not necessarily intended to be merged, but is opened to allow the changes to be discussed and reviewed more easily.

@hawkw hawkw changed the title Eliza/generic service [RFC] Make Service generic Oct 9, 2018
This branch begins adding additional integration tests. In particular,
it adds the following:
 - Additional "hello world" integration tests with request bodies,
   response bodies, and request and response bodies,
 - A `tests/client.rs` module to contain tests for "real" clients with
   mocked-out servers, and,
- A `tests/support.rs` module containing utility code factored out of
  the server tests, which are used in both sets of tests.

I plan to add additional tests in subsequent PRs, but I thought it 
would be best to get these changes merged now.

Test coverage (as reported by [tarpaulin]):
------------------------------------------

Before:
```
$ cargo tarpaulin -p tests --exclude-files tests/* --exclude-files tower-balance/*

Coverage Results
Tested/Total Lines:
src/body.rs: 4/28
src/buf.rs: 13/15
src/client/background.rs: 0/8
src/client/connect.rs: 0/34
src/client/connection.rs: 0/95
src/flush.rs: 35/55
src/recv_body.rs: 2/44
src/server/mod.rs: 57/158
src/service.rs: 0/14

24.61% coverage, 111/451 lines covered
Tarpaulin finished
```

After:
```
$ cargo tarpaulin -p tests --exclude-files tests/* --exclude-files tower-balance/*

Coverage Results
Tested/Total Lines:
src/body.rs: 4/28
src/buf.rs: 13/15
src/client/background.rs: 8/8
src/client/connect.rs: 16/34
src/client/connection.rs: 28/95
src/flush.rs: 35/55
src/recv_body.rs: 24/44
src/server/mod.rs: 57/158
src/service.rs: 0/14

41.02% coverage, 185/451 lines covered
Tarpaulin finished
```

Coverage diff: +16.41%

[tarpaulin]: https://github.com/xd009642/tarpaulin

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw and others added 5 commits October 31, 2018 16:11
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This reverts commit d27d22a.

@carllerche suggested this bound be removed as it is not required
by the trait. Bounding the request body type requires all users
of `HttpService` to _also_ have this bound. This makes using the
trait more difficult.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw
Copy link
Owner Author

hawkw commented Dec 6, 2018

Closing this as the change has already been made upstream.

@hawkw hawkw closed this Dec 6, 2018
@carllerche carllerche deleted the eliza/generic-service branch March 6, 2019 05:12
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