elastic / logstash

Logstash - transport and process your logs, events, or other data
https://www.elastic.co/products/logstash
Other
14.21k stars 3.5k forks source link

support running specs/tests in plugins #2039

Closed colinsurprenant closed 9 years ago

colinsurprenant commented 9 years ago

we should be able to run specs from within a plugin directory without having to install it.

colinsurprenant commented 9 years ago

see PRs #2037 and logstash-plugins/logstash-codec-compress_spooler#1 for an example on how to support specs within a plugin directory.

The basic idea is:

After this, only need to:

bundle install
bundle exec rspec

Ultimately this will require we publish the logstash gem (logstash.gemspec) and this gem will basically be used as dependency within the plugin in developement mode.

electrical commented 9 years ago

There are a few flaws with this idea.

by including the gemspec in the Gemfile it breaks the functionality that we use now in Jenkins. The Gemspec is only used for vendor and publishing tasks. including the gemspec will add more dependencies ( and possibly LS it self )

Also by not using the pluginmanager it self breaks the functionality for processing jar dependencies in the right way and also the use of the vendor.json file.

Also I haven't managed to get it to work using jruby via rbenv. normal ruby did work. ( could be my fault )

colinsurprenant commented 9 years ago

Then we must fix Jenkins. This is the model a gem with a gemspec and a Gemfile for local development works. Obviously, adding the gemspec in the Gemfile pulls all the gem dependencies, that is the purpose, including pulling the whole logstash (as a dependant gem). This is normal because you need to have the logstash codebase to be able to run the specs.

I haven't looked at how Jenkins is setup to run the plugins specs/test but it should be as I described:

jordansissel commented 9 years ago

I understand the use case, I think - being able to run tests from plugins. However, I feel it will be a mistake to publish logstash as a gem. Logstash used to be shipped as a gem, and folks would 'gem install logstash'. I don't want folks being able to 'gem install logstash' because we'll get bug reports about all kinds of nonsense (dependency problems, wrong ruby, etc).

Can we solve 'tests from plugins' without having to publish logstash to rubygems?

colinsurprenant commented 9 years ago

I understand your concern about publishing logstash as a gem, but frankly, the cat is now out of the bag, with everything in logstash now published as a gem. since this is mainly for running tests/specs, I don't think its absolutely necessary to publish as a gem, you can point to a local path or git ref but it would make things simpler. alternatively we could publish it as, for example "logstash-core-dev" and make it obvious in the description that this gem is not a gem to run logstash (we could even print that message at install time). let's followup in #2042 ?

untergeek commented 9 years ago

Just talking crazy here, but would it be so far-fetched to have the logstash package bootstrap itself by installing a logstash-core gem? Then it could exist in ruby gems without breaking anything, but still not have the install package (thereby dodging those trying to bare bootstrap using gem install logstash-core)?

colinsurprenant commented 9 years ago

I think this is not crazy at all and would make the whole setup consistent gem-wise and would fit nicely with #2042

jordansissel commented 9 years ago

@colinsurprenant For myself, I almost never develop plugins against a released version of logstash, I always dev against master and later test against known releases. If we did gems, we still have the unreleased-gem problem that feels poorly solved by bundler.

colinsurprenant commented 9 years ago

@jordansissel the normal workflow when developing gems that depends on local code or on a git ref is to override that gemspec dependency in the Gemfile and running bundle install.

jordansissel commented 9 years ago

before we do git refs, though, we'll really want to shrink the logstash git repo. doing 'git clone' via 'bundle install' fetches 80mb every time.

colinsurprenant commented 9 years ago

Now for automation, if we want to systematically test against master maybe we could have a separate Gemfile.master which would be something like:

source "https://rubygems.org"
gemspec
gem "logstash-core", :github => "elasticsearch/logstash", :ref => "master"

(or a rake task which Jenkins calls that generate such a Gemfile and run bundle install ?)

electrical commented 9 years ago

Okay, so every time we change versions to test against we have to change that? And i want to avoid using git clones for PLugin testing anyway by using Snapshot Tarballs. So that breaks the testing part further....

colinsurprenant commented 9 years ago
electrical commented 9 years ago

Testing against versions does not implies released version, we can also test against certain branches. master, 1.x 1.5 etc

Creating gems for each branch implies generating a gem every time after a commit, that's pretty much a no-go.

colinsurprenant commented 9 years ago

why is that a no-go and why do we want to test all plugins after every branch commit?

electrical commented 9 years ago

Like i explained earlier we will want to test the plugins at every LS core commit to ensure we don't break plugins. atleast then we have a quick response back instead every 24 hours for example.

colinsurprenant commented 9 years ago

I personally think its overkill to test all plugins at every commit but, hey, why not, it's basically a matter a resources.

Why would it be a no-go to create a gem after each commit?

electrical commented 9 years ago

Where would we publish it? rubygems? so we get 1.5.0.2843 at some point?

electrical commented 9 years ago

Btw, testing at every commit is the same what happens with ES and ES clients. i want us to follow the same flow.

colinsurprenant commented 9 years ago

we don't need to publish it, just generate the gem and reference the test against that file.

for testing at every commit, well there are 2 scenarios:

  1. a plugin commit, then we test that plugin against logstash versions
  2. a logstash commit then we test that logstash version agains all plugins.

As I said, if we have the resources to perform all ~160 plugins tests at every logstash commit then fine but I think this will be overkill especially for branches. wouldn't it be better to provide an easy way to manually trigger it? for example, if I push a WIP branch, I don't think there's much value in running all tests for all commits on that branch but at some point I may want to do a sanity check.

electrical commented 9 years ago

Its only meant for branches in the LS repo, master, 1.x 1.5 no feature branches.

jsvd commented 9 years ago

it's now possible to clone a plugin repo, run bundle install && rake vendor && bundle exec rspec