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

Bats tests should use plugins built in the elasticsearch repository #12651

Closed
nik9000 opened this issue Aug 4, 2015 · 12 comments
Closed

Bats tests should use plugins built in the elasticsearch repository #12651

nik9000 opened this issue Aug 4, 2015 · 12 comments
Assignees
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests

Comments

@nik9000
Copy link
Member

nik9000 commented Aug 4, 2015

The bats tests currently install watcher to test installing plugins. It should really install one of the plugins built by the elasticsearch repository. Watcher can test itself. This will also solve the issue of not being able to run the tests properly when there isn't a watcher plugin available for the version of elasticsearch we're building. There will always be an appropriate version of the locally built plugins around because we build it as part of the build.

@tlrx
Copy link
Member

tlrx commented Aug 4, 2015

Sounds like a good idea!

@nik9000 nik9000 self-assigned this Aug 4, 2015
@nik9000
Copy link
Member Author

nik9000 commented Aug 4, 2015

I'm so going to do this once we merge #12646

@clintongormley clintongormley changed the title [Packaging] Bats tests should use plugins built in the elasticsearch repository Bats tests should use plugins built in the elasticsearch repository Aug 5, 2015
@clintongormley clintongormley added >test Issues or PRs that are addressing/adding tests :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts labels Aug 5, 2015
@jaymode
Copy link
Member

jaymode commented Aug 11, 2015

I like this idea, but do we have a plugin in this repository that has a bin and config dir inside it? I think some of the tests look for that so we may need to add a test one with those directories or something

@uboness
Copy link
Contributor

uboness commented Aug 11, 2015

+1 on ditching watcher... and +1 on a plugin with config and bin dirs

@dadoonet
Copy link
Member

I guess we can add that to the jvm example plugin: https://github.com/elastic/elasticsearch/tree/master/plugins/jvm-example
?

@nik9000
Copy link
Member Author

nik9000 commented Aug 11, 2015

I like this idea, but do we have a plugin in this repository that has a bin and config dir inside it? I think some of the tests look for that so we may need to add a test one with those directories or something

I guess we can add that to the jvm example plugin: https://github.com/elastic/elasticsearch/tree/master/plugins/jvm-example

I've already started....

@nik9000
Copy link
Member Author

nik9000 commented Aug 11, 2015

#12787

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Aug 14, 2015
This will be useful in testing the plugin installer.

Relates to elastic#12651
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Aug 14, 2015
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Aug 17, 2015
@nik9000 nik9000 added the v2.0.0 label Aug 17, 2015
@nik9000
Copy link
Member Author

nik9000 commented Aug 17, 2015

Ok! All done. The tests all use the jvm-example plugin and no longer try to use watcher. They deserve some more refactoring and we should really be installing all the plugins to make sure they make it. But I'll do that under #12717.

@nik9000 nik9000 closed this as completed Aug 17, 2015
@nik9000
Copy link
Member Author

nik9000 commented Aug 17, 2015

Silly closed icon! I didn't reject this issue! I fixed it in four different pull requests!

@dadoonet
Copy link
Member

@nik9000 Why did not you push your change to 2.0 branch? On purpose?
cc @clintongormley

@nik9000
Copy link
Member Author

nik9000 commented Aug 19, 2015

@nik9000 Why did not you push your change to 2.0 branch? On purpose?

Do we need it in 2.0? I was going by the mantra of "don't push anything we don't need there". I think I have quite a few bats changes just in master that aren't in 2.0 yet. I can push them if you think its important. For the most part it'd probably work just fine to test 2.0-beta-whatever changes by staging them in the testroot or changing the elasticsearch.verion in master.

@dadoonet
Copy link
Member

I don't know. While merging some of my PR from master to 2.0 branch, I just discovered that was not merge and that 2.0 branch still wants to install shield for vagrant tests.

Not a big deal to me. Was just asking if it was intentional not to merge it or just that you forgot :)

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Sep 1, 2015
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Sep 1, 2015
This will be useful in testing the plugin installer.

Relates to elastic#12651
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests
Projects
None yet
Development

No branches or pull requests

7 participants