lipanski / mockito

HTTP mocking for Rust!
MIT License
670 stars 57 forks source link

dynamic ports #25

Closed jupp0r closed 5 years ago

jupp0r commented 7 years ago

Using a static port for mockito makes integration tests brittle for a number of reasons:

Mockito could bind to port 0, which makes the OS choose a free port to bind on, and then let the test know which port was chosen. This would also enable parallel test execution which each test having their own mockito instance on their own port.

Thoughts, ideas?

lipanski commented 7 years ago

I think this is doable nowadays (wasn't possible with the mockito architecture of several months ago). initially I started out with port 0, but had to pin it at some point. will take another look.

lipanski commented 7 years ago

This would also enable parallel test execution which each test having their own mockito instance on their own port.

@jupp0r not sure about that part though. how would you tell every individual test which port to call? I thought about thread IDs as a way to associate tests with a particular set of mocks, but the moment you're application code uses threads itself, you're lost.

parallel test execution is possible with mockito as we speak, but only as long as you're mocks don't collide (or interfere with other tests)

otavio commented 6 years ago

parallel test execution is possible with mockito as we speak, but only as long as you're mocks don't collide (or interfere with other tests)

Agreed but it is suboptimal as running the tests in parallel does provide a great speed up.

lipanski commented 6 years ago

@otavio any suggestions would be welcome, I was kind of stuck here :)

running the tests in parallel does provide a great speed up

in percentual values, for sure. but overall mockito is pretty fast and Rust is fast as well. are you experiencing slow test runs and if so would you care to share some samples and/or numbers?

jupp0r commented 6 years ago

@lipanski : I'm not sure what you are hinting at. Any large code base with good test coverage will have at least multi-minute test suites, even in "fast" languages. The link you supplied mentions ~4ms for creating a test mock. A C++ code base I work on at work has 17000 test cases, that makes more than a minute just when creating one mock per test case. Running tests in parallel (especially on multi-core workstations) can make the difference between waiting hours or minutes, really.

otavio commented 6 years ago

I agree with @jupp0r here.

As a stopgap solution I am using following build.rs script inside my project:

fn main() {
    // This forces the tests to run in a single thread. This is
    // required for use of the mock server[1] otherwise one test impacts
    // the other.
    //
    // 1. https://github.com/lipanski/mockito/issues/25
    println!("cargo:rustc-env=RUST_TEST_THREADS=1");
}

This is far from optimal but at least allow our coworkers to trigger the tests using cargo test without bad surprises.

lipanski commented 6 years ago

@jupp0r the 4.8ms were from 0.4.2 (I highlighted the wrong line). the last version for which I ran the benchmarks (0.8.0) clocks in at 17μs (mileage may vary of course).

ncoish commented 6 years ago

Is this feature still under consideration? I would love to use this crate for testing, but setting RUST_TEST_THREADS=1 for my project would get me killed by my team members.

otavio commented 6 years ago

@tridius as we discussed here, there is no known solution for this problem.

How we'd "tell" a test which port it should use? Specifically, how we will have local server definition for each test.

I know it can be done, specifically having a return of mock method telling where it is going to run but it breaks existing users.

ncoish commented 6 years ago

Yeah I was thinking have the mock function just return the port as a part of the returned Mock struct. If it breaks existing users then it's just a major version increase no? So this is possible, but a longer term feature that would get grouped in to a bigger release?

estin commented 6 years ago

For me workaround for multiple threads case is unique urls for each test:

P.S. Sorry for my poor English

otavio commented 6 years ago

@estin using individual port or unique URLs work. It is mostly a matter of deciding which route to take. Essentially, it is a @lipanski call ;-)

lipanski commented 6 years ago

@otavio

specifically having a return of mock method telling where it is going to run but it breaks existing users.

how do you set the host in your projects? do you expose a (public) setter? I've been mostly using constants or static values or at least I've never had the need for a public setter. if your project exposes public setters I guess I can run one server per mock (or maybe introduce a method/parameter to group several mocks under one server) and have the mock struct (or this other method) expose the mockito server address.

breaking compatibility wouldn't be a major problem if it's for a nice, user-friendly API.

@estin

are you simply careful not to mock same requests with different responses or not to collide your requests across different tests or is there something else?

estin commented 6 years ago

are you simply careful not to mock same requests with different responses or not to collide your requests across different tests or is there something else?

Not to collide requests across different tests. One rule for tests is "encapsulate all request-response checks for one http service in one test". In my app I can do it with minimal effort, the app logic allow it. And yes this "method" can not be applied for other apps with complex logic.

And mockito is fine and useful for my cases

otavio commented 6 years ago

how do you set the host in your projects? do you expose a (public) setter? I've been mostly using constants or static values or at least I've never had the need for a public setter. if your project exposes public setters I guess I can run one server per mock (or maybe introduce a method/parameter to group several mocks under one server) and have the mock struct (or this other method) expose the mockito server address.

breaking compatibility wouldn't be a major problem if it's for a nice, user-friendly API.

In our specific case, we have a settings struct that holds the server address. Currently, I initialize it as a default value but I can easily make a macro to initialize it and change the value to comply with the one returned by the mock.

lipanski commented 6 years ago

Please check https://github.com/lipanski/mockito/releases/tag/0.11.0

It doesn't solve this issue entirely, but it removes the need for appending --test-threads=1 and, more importantly, tests that don't use mocks can be run in parallel.

kornelski commented 5 years ago

I've managed to implement dynamic ports: #58

lipanski commented 5 years ago

Released in 0.15.0.