search

magnusbaeck / logstash-filter-verifier

Apache License 2.0
192 stars 27 forks source link

Parallel execution of multiple test cases #15

Closed breml closed 7 years ago

breml commented 7 years ago

Implements the usage of logstash-input-unix as input, which allows to use a single Logstash instance for the execution of multiple test case files, which greatly shortens the duration for the test execution.

To enable this feature, run logstash-filter-verifier with the command line flage --sockets. This feature does only work on systems, which do support unix domain sockets.

The logstash-input-unix plugins adds the two fields host and path. Therefore these fields should be ignored in the test cases.

This implementation does need at least Go 1.8beta1, as the function net.SetUnlinkOnClose is used, which is introduced with Go 1.8.

Fixes #14 May help for #11

breml commented 7 years ago

@magnusbaeck Yes, this is right, Logstash does not shutdown, until the last unix socket is closed. And after the last unix socket is closed, the events in the Logstash filter pipeline are processed, before Logstash exits.

I fixed the typo and the ts.sender = nil together with some minor go lint findings.

breml commented 7 years ago

@magnusbaeck I just updated my branch with the requested changes. There are still no tests. I hope I find some time later this week to add some tests.

breml commented 7 years ago

Just a short update, I have started to work on the tests, but I am not yet ready to push.

breml commented 7 years ago

@magnusbaeck added a test for the parallel execution. I am also thinking about adding build flags, so the unix domain socket part would only be included, if one builds logstash-filter-verifier on a system, which does actually support unix domain sockets.

magnusbaeck commented 7 years ago

Code looks good, thank you! Just two things:

breml commented 7 years ago

I am fine with not yet releasing this feature, until Go 1.8 will be released, because this will be any day.

The solution for the problem with "Testcase failed, continuing with the rest: Expected 2 event(s), got 1 instead." is to add "codec": "line" to your test case files. This is the default codec for logstash-input-unix, but it is changed by logstash-filter-verifier if not explicitly set (https://github.com/magnusbaeck/logstash-filter-verifier/blob/master/testcase/testcase.go#L112). With this change it should work for you even without the modified/patched version of logstash-input-unix. With the patched version the warning Workaround for IOError in unix.rb on stop, process result anyway. (see https://github.com/logstash-plugins/logstash-input-unix/pull/18) also vanishes.

I guess this should be mentioned in the README.md (where this feature is currently completely missing).

breml commented 7 years ago

Added some text about --sockets to the README.md. This could also be the place, where this feature would be marked as experimental.

breml commented 7 years ago

@magnusbaeck Go 1.8 was released yesterday, updated the PR (especially the .travis.yml). Do you think, we should add a hint to Go 1.8 in the README.md as well?

Could you verify that it is working if the codec is set to line?

magnusbaeck commented 7 years ago

Go 1.8 was released yesterday, updated the PR (especially the .travis.yml). Do you think, we should add a hint to Go 1.8 in the README.md as well?

Yes, that's a good idea.

Could you verify that it is working if the codec is set to line?

I'll do that this weekend and make sure this PR is merged if everything checks out.

breml commented 7 years ago

Thanks for merging and the improvements you added yesterday (removal of path field).

magnusbaeck commented 7 years ago

No, thank you! This is a huge performance improvement. Is there a reason not to make the line codec the default? Having to adjust the test case files to make --sockets work doesn't feel right.

breml commented 7 years ago

Good question. Moved this discussion to #24.