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

Clean up Client and Server integration tests #1675

Open
seanmonstar opened this issue Oct 18, 2018 · 9 comments
Open

Clean up Client and Server integration tests #1675

seanmonstar opened this issue Oct 18, 2018 · 9 comments
Labels
A-tests Area: tests. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@seanmonstar
Copy link
Member

Both the client and server tests (tests/client.rs and tests/server.rs) started with small abstractions, but they weren't flexible enough, and so as more complicated cases were needed, the tests just got more and more complicated, with a lot of repetition. It'd be huge to find a way to refactor them.

@seanmonstar seanmonstar added E-easy Effort: easy. A task that would be a great starting point for a new contributor. A-tests Area: tests. labels Oct 18, 2018
@JeffLabonte
Copy link
Contributor

I will take this issue and work on it!

@JeffLabonte
Copy link
Contributor

Would you have something in mind as well? @seanmonstar

@tiendq
Copy link

tiendq commented Jun 24, 2019

@seanmonstar is it still open? :)

@seanmonstar
Copy link
Member Author

Yes, this is still open because the integration tests feel very messy, and there's a lot of repeated code. I'm sorry I haven't been able to provide much guidance on how to improve them, if I thought about it a lot, I'd probably fix them myself :)

@tiendq
Copy link

tiendq commented Jun 25, 2019

Well, if it's very messy so we could improve it a little each PR, for example let's remove (as many as possible) repeated code first. If you feel ok with this approach I could try it :) (hopefully it's easy as labelled :D).

@ryanyz10
Copy link

I'd like to try and tackle this one if it hasn't been done yet! The tests seem quite long though...

@seanmonstar
Copy link
Member Author

Yea, it's not done yet. Admittedly, this is between an easy and medium task. For example, in tests/client.rs there is a few submodules testing different implementations or parts of the client API, and most are using a lot of the same code copied and pasted but then ever slightly modified. So it makes the files long, and more tedious to adjust. It'd be best if we could break it up into multiple tests/client_blah.rs files, separated by main "concerns", and with the various duplicate code cleaned up into smaller helpers.

@ryanyz10
Copy link

Ok great, I'll slowly work on this then.

alexs-sh added a commit to alexs-sh/hyper that referenced this issue Dec 4, 2022
This is a part of hyperium#1675 issue. It slightly reduces the number of
duplicated code by using one common function for init a logger and
creating a Tokio server.
alexs-sh added a commit to alexs-sh/hyper that referenced this issue Dec 4, 2022
This is a part of hyperium#1675 issue. It slightly reduces the number of
duplicated code by using one common function for init a logger and
creating a std server.
@alexs-sh
Copy link
Contributor

alexs-sh commented Dec 4, 2022

Hi @seanmonstar
Could you please take a look at #3072

This PR doesn't provide any significant changes but allows to make one step in decreasing code duplication. If it's the wrong way, please, just reject the PR. If it brings something useful, then I can continue with it.

Thanks

seanmonstar pushed a commit that referenced this issue Dec 5, 2022
This is a part of #1675 issue. It slightly reduces the number of
duplicated code by using one common function for init a logger and
creating a Tokio server.
alexs-sh pushed a commit to alexs-sh/hyper that referenced this issue Dec 11, 2022
This is a continuation of work on hyperium#1675 issue. It slightly reduces the
number of duplicated code by using one common function for init a logger
and creating tcp listener for tests.
alexs-sh added a commit to alexs-sh/hyper that referenced this issue Dec 11, 2022
This is a continuation of work on hyperium#1675 issue. It slightly
reduces the number of duplicated code by using one common function for
init a logger and creating tcp listener for tests.
alexs-sh added a commit to alexs-sh/hyper that referenced this issue Dec 11, 2022
This is a continuation of work on hyperium#1675 issue. It slightly
reduces the number of duplicated code by using one common function for
init a logger and creating tcp listener for tests.
seanmonstar pushed a commit that referenced this issue Dec 12, 2022
This is a continuation of work on #1675 issue. It slightly
reduces the number of duplicated code by using one common function for
init a logger and creating tcp listener for tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tests Area: tests. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Development

No branches or pull requests

5 participants