Skip to content

port fix for windows command line options to master #16129

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.

Sign up for GitHub

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

Jump to bottom
Closed
brwe opened this issue Jan 20, 2016 · 8 comments
Closed

port fix for windows command line options to master #16129

brwe opened this issue Jan 20, 2016 · 8 comments
Assignees
@brwe
Labels
>bug :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team

Comments

@brwe
Copy link
Contributor

brwe commented Jan 20, 2016

This pr #15320
cannot be easily ported to master because we need to translate mvn/ant to gradle which might take me a little so I open an issue here.
@brwe brwe added v5.0.0-alpha1 :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >bug labels Jan 20, 2016
@rjernst
Copy link
Member

rjernst commented Jan 20, 2016

I don't think the change will be that difficult, but I would also wait until I've finished refactoring ES into a fixture. Part of that is to use ProcessBuilder instead of ant exec, at which point we will no longer use the pidfiles for correct test execution, and the command line qa test you have here can set the options and check the pid file itself.

@brwe
Copy link
Contributor Author

brwe commented Jan 20, 2016

I would also wait until I've finished refactoring ES into a fixture

ok

@clintongormley
Copy link
Contributor

@rjernst does this still need doing?

@jasontedor
Copy link
Member

does this still need doing?

The fix has been done in master in e32da55 by @ywelsch after the command-line parsing refactoring in #17088 but the tests have not been forward-ported.

@clintongormley
Copy link
Contributor

@brwe would you be able to port the tests to master please?

@brwe
Copy link
Contributor Author

brwe commented May 9, 2016

ok

brwe added a commit to brwe/elasticsearch that referenced this issue May 18, 2016
@brwe
[TEST] test command line options
1f9b5e8 
This commit adds a test for command line options -p, --version and --help

closes elastic#16129
@brwe brwe mentioned this issue May 18, 2016
Closed
@brwe
Copy link
Contributor Author

brwe commented May 19, 2016

We discussed it in #18437 and came to the conclusion that might make more sense to invest in bats tests for windows (#18475) instead of having this one test in gradle or the likes.

@brwe
Copy link
Contributor Author

brwe commented May 25, 2016

superseded by #18475

@brwe brwe closed this as completed May 25, 2016
@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
Assignees

@brwe brwe

Labels
>bug :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[TEST] test command line options brwe/elasticsearch
5 participants
@clintongormley @rjernst @mark-vieira @brwe @jasontedor