make testing better mimic reality for securitymanager#10965
Merged
rmuir merged 5 commits intoelastic:masterfrom May 5, 2015
Merged
make testing better mimic reality for securitymanager#10965rmuir merged 5 commits intoelastic:masterfrom
rmuir merged 5 commits intoelastic:masterfrom
Conversation
Contributor
|
good catch rob! LGTM |
Contributor
Author
|
@rjernst @s1monw i pushed a new commit: rmuir@072b902 |
Member
There was a problem hiding this comment.
can the 2 tests calling this with false just override java.io.tmpdir for the test? then no need for a new boolean?
Contributor
|
still LGTM |
Member
|
LGTM, I pushed a change to remove the flag for adding tmp dir permissions. |
rmuir
added a commit
that referenced
this pull request
May 5, 2015
make testing better mimic reality for securitymanager
Contributor
Author
|
Thanks for that test improvement @rjernst |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The two things are not the same today: so in tests we should use the same technique, and install policy etc the same way. But today tests do things differently than bin/elasticsearch, like use a java.security.policy system property. And this hides bugs!
Once its fixed to work the same way, we see the bugs, the java.io.tmpdir sysprop substitution does not work (#10925 this is some chicken/egg problem in the jvm). We don't use this method and instead dynamically generate it just like other paths in environment.
Second, today things like SSL are not going to work at all. That is because we don't have any grants for system permissions for jdk codesources (but when you set a sysprop, like tests were doing, it sucks in default files).
Finally, this has the advantage that now tests too, don't depend on the environment, like policy configurations on the machine that someone may have changed.