Fix flakiness of engine tests

[robingustafsson]

There are a couple of test cases for the engine that are flaky. We need to fix them as they generate a lot of false positive CI builds.

The test cases are:

• TestEngineRun:collects samples: this test has two wrong assumptions. The first is that the cutoff in core/local/local.go will be set before the ctx is signalled Done resulting in the second sample to be appended to the samples slice in the mocked "RunOnce()" function (basically a race condition). The second is that <-ctx.Done() will be selected before case samples := <-vuOut in core/local/local.go:Run() which is not always the case (non-determinism of select), and can cause all samples from the mocked "RunOnce()" function to be accepted. When either of these two happens there's a "wrong number of samples" error being generated.
• TestEngineCollector: There seems to be a race condition here, the test sometimes errors out with a "panic: attempted to collect while not running" (from Dummy collector).

Beyond these two, CircleCI builds also fail sometimes when running tests dependent on external sites like httpbin.org, see #464.

[robingustafsson]

First one (TestEngineRun:collects samples) fixed in 48f2c40

[antekresic]

How about using this replace httpbin.org dependencies?
https://github.com/ahmetb/go-httpbin

Or maybe just keep it simple and use httptest package?

[antekresic]

@robingustafsson it seems everything from this issue has been taken care of except the TestEngineCollector test. I have encountered this issue yesterday and it seems the issue is in how the Collect function is implemented in the test Collector. There is a check for the running flag which is inconsistent with other Collector implementations since they don't do such a thing.

I guess we have to sync up the test with actual Collector implementations in order to get this sorted out. I'm willing to do take care of this one once we agree on the solution here.

[na--]

Just making a note here that in #553 we discussed that the dummy collector has to be refactored, probably it should use a Sample channel instead of a slice. When the context channel is closed, Run() can close the Samples channel and exit, all following Collect() calls will panic - just like the current behaviour, but better :)
Edit: after a second look, I gave up on the channel idea, see rationale in #561