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

HTTPS Support #69

Closed
wants to merge 3 commits into from
Closed

HTTPS Support #69

wants to merge 3 commits into from

Conversation

msvab
Copy link
Contributor

@msvab msvab commented Mar 15, 2014

Hi, I really enjoy using Moco, however on my current project we needed HTTPS support, so I tried to put something together. Please let me know, if you think it's good enough to be merged in. If not, I'm happy to change it as necessary.

Thanks!

@garrettheel
Copy link
Contributor

I think it looks great and would definitely be useful to Moco. A couple of improvements you could make:

  • No hardcoded file paths, like /cert.jks
  • Add settings to make the https server more configurable (e.g allow for server rejecting certificate)

@msvab
Copy link
Contributor Author

msvab commented Mar 16, 2014

I agree that I should have made the certificate configurable in first place. So I've changed that.

dreamhead added a commit that referenced this pull request Mar 19, 2014
@dreamhead
Copy link
Owner

Thank you guys! I've merged it into master.

One thing I could change for this implementation is that I hope API could be like this

HttpsServer server = httpsServer(12306);
Runner runner = runner(server);
runner.start();

Do you like this idea?

@msvab
Copy link
Contributor Author

msvab commented Mar 19, 2014

I guess that would make sense. Have you ever considered merging the Runner and HttpServer? As a user of the library, I find it quite confusing that there are two classes I have to interact with, rather than just one. So far I've always used Moco from JUnit tests and to make the tests less verbose I've always wrapped Moco in another class, so I can interact with just one object.

Example would be:

package stub;

import com.github.dreamhead.moco.HttpServer;
import com.github.dreamhead.moco.Runner;
import com.github.dreamhead.moco.monitor.Slf4jMonitor;
import lombok.Delegate;
import org.junit.rules.TestWatcher;
import org.junit.runner.Description;

import static com.github.dreamhead.moco.Moco.httpserver;
import static com.github.dreamhead.moco.Runner.runner;

public class SomeStub extends TestWatcher {

    private Runner runner;
    private @Delegate HttpServer server;

    @Override protected void starting(Description description) {
        if (runner == null) {
            server = httpserver(9010, new Slf4jMonitor());
            runner = runner(server);
            runner.start();
        }
    }

    @Override protected void finished(Description description) {
        runner.stop();
    }
}

@dreamhead
Copy link
Owner

In my original design, HttpServer is the class which is used to describe server configuration. The default usage is running with closure. I added runner as another way to run. It's a good suggestion and I'll consider if I have an appropriate way to avoid misuse it, e.g. setup server after server running. I would like to hear your suggestion.

@msvab
Copy link
Contributor Author

msvab commented Mar 22, 2014

I guess it makes sense to support both ways of running the server. For simple tests, the closure makes sense and with the Java 8, the verbosity should go down a bit as well I guess.

I personally prefer the Runner, as I'd like to start/stop the server outside of my test (e.g. in setup/teardown methods), so the tests don't contain any boilerplate code. And it would be great to be able to change server setup while it's running. In case there are multiple tests within a single test class (there usually are), I think it would be nice to keep the server running and each test would just setup it's required calls.

But this is quite a big change from your design I guess, what do you think?

@dreamhead
Copy link
Owner

You are right. It would be better if the configuration can be changed on runtime and it will be a big change. If Moco can do this, it should be easy to implement JUnit Rule which could provide better tooling integration and maybe we don't need to use closure.

Actually it is already in my personal plan, I just don't have enough time to do that. Welcome your further idea and contribution.

@msvab
Copy link
Contributor Author

msvab commented Mar 23, 2014

Great! I have a few busy weeks ahead, but I'll see if I can find some time for this.

Thanks

@msvab msvab closed this Mar 23, 2014
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.

None yet

3 participants