HTTP: Integration Testing

[jamespic]

I've been brought up to believe that whenever you fix a bug, you should add a test for it, so it doesn't re-surface. I didn't include a test with issue #1845, because it was hard to test at a component level, and that's been bothering me.

This patch includes a test for #1845 (in HttpIntegrationSpec). It uses WireMock to simulate a system-under-test, and initialises just enough of Gatling to test it. There's also a re-usable support class, to enable this integrated style of testing.

I can't decide if this is a good approach or not. More testing's good, but it's adding complexity.

Thoughts?

[slandelle]

First time I hear about WireMock. Do you use frequently it? Could be the piece we were missing for properly having integration tests.

[slandelle]

@pdalpra @nremond WDYT?

[slandelle]

So this is a mistake. Let's fix this!

[jamespic]

TBH, I hadn't used WireMock before. I picked it as it seemed reasonably simple, and well maintained. But I'm happy to switch to something else, if it fits the bigger picture.

[nremond]

Is this pertinent ?

[jamespic]

It's not at all pertinent. I just hadn't gotten round to disabling it in my IDE. I'll fix it.

[nremond]

I really like the concept here. Then, same as @slandelle, first time I here from WireMock. It looks to be based on Jetty, so, it's may be a HTTP-stack that we can trust.
@jamespic: any idea on the execution time of your test? the increased memory usage of SBT ?

[jamespic]

According to VisualVM, the test takes 4038ms on my system. Of that, 1463ms is WireMock setup, 500ms is test execution, and the rest is Gatling setup. There's only one test - I don't know how much time we'd save on subsequent tests. I'll get some memory stats.

[jamespic]

I added a few duplicate tests to measure how subsequent tests perform (after JVM warm-up). After the first test, they take around 200ms each.

I haven't been able to get any hard numbers on memory usage. I can say that the increase doesn't seem to be significant - on my machine, peak memory usage is around 200MB, whether I include HttpIntegrationSpec or not.

[nremond]

Thanks for these details.
LGTM.

[slandelle]

WireMock looks like a good idea, let's go for it.

The only thing that bothers me here is actually ActorSupport, where configuration set up has already changed quite a few times. There should be a way to pass the desired configuration, right? Also, def of is never used, right? WDYT?

[slandelle]

What if we have:

def apply[R: AsResult](newConfig: GatlingConfiguration)
                                   (f: TestKit with ImplicitSender => R): Result = ???

def apply[R: AsResult](f: TestKit with ImplicitSender => R): Result =
apply(GatlingConfiguration.fakeConfig(Map("gatling.data.writers" -> "console")))(f)

[jamespic]

@slandelle The of method is used. I added it when I refactored RedisFeederTest to use ActorSupport. RedisFeederTest didn't actually use the TestKit instance that apply provided, so I figured the test was more readable without it. I wanted to make it another apply method, but type inference got confused by all the apply methods, hence the name of.

Agree on the config issue. I'll fix it.

[slandelle]

The of method is used. I added it when I refactored RedisFeederTest to use ActorSupport
You're right, sorry for missing it

Can you then rebase your PR so it can be merged, please? Could you also fixup your commits into a a single one?

[slandelle]

Thanks!

[slandelle]

I realize that wiremock depends on Jetty 6?! WTF!

[nremond]

It's what I said last week ;)

[slandelle]

Didn't realize that. That doesn't make me very confident with this lib...

[slandelle]

The problem is not Jetty itsef, Jetty is great. The problem is Jetty 6! Current version is Jetty 9. Jetty 6 was last released in 2007!

[nremond]

Oh indeed. Missed that.

[slandelle]

I just asked directly on the wiremock project: https://github.com/tomakehurst/wiremock/issues/137

What gets me concerned here is the risk that we might have some failing tests, not because of our code but because of some long time fixed bug in Jetty.

In AHC, we directly use Jetty. Sure less sexy but working well. Note that I uncovered some issues on the AHC side when I upgraded the tests from Jetty 8 to 9.

[tomakehurst]

Hi,

I've just seen the above issue logged against WireMock, so I thought it'd be worth chiming in.

I've not upgraded Jetty so far because
a) there's never been a particularly good reason to do so - Jetty 6 has done the job just fine, and
b) WireMock contains a couple of hacks for doing fault injection that depend on specific Jetty internals, so the job of upgrading it isn't all that trivial.
It's certainly not because the project is neglected!

Having said all that a bug has come up recently which I suspect may be Jetty's fault, so I'm looking at moving to Jetty 8. I've been asked not to upgrade to 9 for the time being because apparently it won't work in the Android VM, whereas 8 will.

I'm curious - is there a specific performance/concurrency concern you have with this Jetty version? I'd be happy to expedite the upgrade if it'll enable better perf/load testing.

Thanks,
Tom

[slandelle]

Hi @tomakehurst
Sorry, I first answered in the issue I opened on wiremock before noticing you had answered here too.

[slandelle]

Basically, we're in the process of selecting a library for building our integration tests. Wiremock sure is active, the API looks nice, but Jetty 6 is quite old.

[tomakehurst]

So would only Jetty 9 suit your needs, or would 8 be OK?

[slandelle]

I guess we could cope with Jetty 8.
I don't know which JDK version you target. Jetty 9 requires JDK7.

Regarding the Android compatibility, I can just give you my personal opinion: trying to have a HTTP library that both targets regular Java and Android is a lost cause. Regular HTTP libraries tend to use NIO features and advanced techniques such as Unsafe that don't exist/work well on Android.

As a consequence, Android users start growing their own libraries, such as OkHttp. The Jetty guys maintain a Jetty fork dedicated to Android: https://code.google.com/p/i-jetty.

[tomakehurst]

OK, that's useful to know. I've never done more than a bit of hobbyist Android development so I'm only going on what other WireMock users have told me.

A while back I started a branch to look at the feasibility of supporting multiple web server libraries (which I wish I'd merged while it was still easy!). Perhaps I should revive that idea, then you can pick your server based on whatever compatibility, performance, startup time etc. tradeoffs suit your use case.

It's still possible to run WireMock on JDK 6, so I'd need to think carefully about dropping support for that. But given that JDK 7 has been around for nearly 3 years now it's probably not an unreasonable thing to do.

[slandelle]

Gatling 2.0 will still JDK6 compatible, but 2.1 will target JDK7. JDK6 has reached eol last december and JDK7 will reach eol april next year.

[tomakehurst]

Will you be writing integration tests for Gatling 2.0 with WireMock (or something like it), or starting from 2.1?

[slandelle]

This Pull Request is our first WireMock based integration test and we merged it.
If we're to go with WireMock, we'll generalize in Gatling 2.1.

[tomakehurst]

OK, so JDK7+ support would suffice.

[slandelle]

Sure