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

Use jvm-example for testing bin/plugin #12895

Merged
merged 2 commits into from Aug 17, 2015

Conversation

Projects
None yet
4 participants
@nik9000
Contributor

nik9000 commented Aug 14, 2015

Related to #12651

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Aug 14, 2015

Contributor

Ping @tlrx for review - I'll be adding another pr to handle the TODO.

Contributor

nik9000 commented Aug 14, 2015

Ping @tlrx for review - I'll be adding another pr to handle the TODO.

@rjernst

View changes

Show outdated Hide outdated qa/vagrant/src/test/resources/packaging/scripts/25_tar_plugins.bats Outdated
@rjernst

View changes

Show outdated Hide outdated qa/vagrant/src/test/resources/packaging/scripts/25_tar_plugins.bats Outdated
@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Aug 14, 2015

Member

LGTM, I would just remove all the commented out file checks for shield.

Member

rjernst commented Aug 14, 2015

LGTM, I would just remove all the commented out file checks for shield.

@s1monw

View changes

Show outdated Hide outdated qa/vagrant/src/test/resources/packaging/scripts/25_tar_plugins.bats Outdated
@s1monw

View changes

Show outdated Hide outdated qa/vagrant/src/test/resources/packaging/scripts/25_tar_plugins.bats Outdated
@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Aug 16, 2015

Contributor

Rebased, added tests for plugins with bin directory, and removed all mention of shield. Sorry for the rebase - I had to do it to get access to a plugin with a bin directory.

Contributor

nik9000 commented Aug 16, 2015

Rebased, added tests for plugins with bin directory, and removed all mention of shield. Sorry for the rebase - I had to do it to get access to a plugin with a bin directory.

@tlrx

This comment has been minimized.

Show comment
Hide comment
@tlrx

tlrx Aug 17, 2015

Member

The changes LGTM but bats tests are failing at various stages on my laptop. I guess this is due to recent changes and not this PR specifically.

Member

tlrx commented Aug 17, 2015

The changes LGTM but bats tests are failing at various stages on my laptop. I guess this is due to recent changes and not this PR specifically.

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Aug 17, 2015

Contributor

The changes LGTM but bats tests are failing at various stages on my laptop. I guess this is due to recent changes and not this PR specifically.

Hmm - they work for me:

vagrant destory -f
mvn clean install -DskipTests
mvn -pl qa/vagrant -Dtests.vagrant verify

and it all passes. That first line isn't usually required but if things are flakey its best to just rebuild the VMs....

Contributor

nik9000 commented Aug 17, 2015

The changes LGTM but bats tests are failing at various stages on my laptop. I guess this is due to recent changes and not this PR specifically.

Hmm - they work for me:

vagrant destory -f
mvn clean install -DskipTests
mvn -pl qa/vagrant -Dtests.vagrant verify

and it all passes. That first line isn't usually required but if things are flakey its best to just rebuild the VMs....

@nik9000 nik9000 referenced this pull request Aug 17, 2015

Merged

Clean up the tar tests #12903

nik9000 added some commits Aug 14, 2015

Add tests for plugins with bin directory
Also removes all mention of shield:
```bash
$ find $BATS -type f -exec grep -Hi shield {} \;
$
```
@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Aug 17, 2015

Contributor

Ok - this has enough LGTMs that I've rebased it and will merge it to master. I'll help @tlrx work through vagrant issues when our days overlap better.

Contributor

nik9000 commented Aug 17, 2015

Ok - this has enough LGTMs that I've rebased it and will merge it to master. I'll help @tlrx work through vagrant issues when our days overlap better.

nik9000 added a commit that referenced this pull request Aug 17, 2015

Merge pull request #12895 from nik9000/config_tests
Use jvm-example for testing bin/plugin

@nik9000 nik9000 merged commit ded5d74 into elastic:master Aug 17, 2015

1 check passed

CLA Commit author has signed the CLA
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment