Some fixes to make tests more robust around jvm_options.

Review Request #3706 — Created April 16, 2016 and submitted

stuhood, zundel

This came out of a discarded change that actually set the jvm_options
to be non-empty, which exposed various issues in tests that assume
that the default jvm_options are empty.

This commit salvages the fixes for those issues, so that if in the future
we do change the jvm_options defaults, the tests will continue to work.

Note: this change does not modify the current jvm_options default, which
remains empty. It merely enables us to do so in the future.

CI passes:

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
  2. I don't see a name collision, why did you change the name here? The convention in our test naming has been to just drop the test_ prefix from the name of the source file.

    I like having a consistent convention. It makes it convenient when I am trying to hand write the spec on the command line.

    1. Indeed, that convention is why I changed the name. The source file is so the target name should be thrift_linter.

    2. .backwards reading diff, Sorry

  3. I will concede to removing this, but can leave it in a separate PR so that if it does end up causing problems for someone we can find it with git bisect?

    1. Ooops, that actually wasn't supposed to be removed here. This was intended to just salvage the changes necessary to get the tests to pass with non-empty default jvm_options. Damn jetlag... Restored.

  2. Add a comment to specify why this is here since it isn't actaully used for anything yet?

    1. It is used in, but I'll add a comment to remind the reader of this fact.

  1. Ship It!
Review request changed

Status: Closed (submitted)

Change Summary:


  1. Submitted, thanks Eric!