https://pantsbuild.org/ logo
Powered by
#general
  • h

    happy-kitchen-89482

    08/03/2015, 8:16 PM
    but no, you don't need to pass in default options in that case
  • h

    happy-kitchen-89482

    08/03/2015, 8:16 PM
    BaseTest does the right thing
  • h

    happy-kitchen-89482

    08/03/2015, 8:16 PM
    @rough-minister-58256: You shouldn't need to explicitly pass in default values for options
  • e

    enough-analyst-54434

    08/03/2015, 8:17 PM
    You definitely do today when building a fake optionvaluecontainer for a Subsystem constructor
  • e

    enough-analyst-54434

    08/03/2015, 8:17 PM
    if doing so directly
  • e

    enough-analyst-54434

    08/03/2015, 8:18 PM
    ... but you're talking about using BaseTest I think
  • e

    enough-analyst-54434

    08/03/2015, 8:26 PM
    By 
    --jvm-debug --jvm-debug-port=1234
    I meant having to use that vs just 
    --jvm-debug-port=1234
    - the latter is all you should need to say when you want to debug on a non-standard port
  • f

    fast-megabyte-33579

    08/03/2015, 8:31 PM
    hmm. Just having the latter option on its own doesn't have any effect does it (it doesn't error out and say "you can't turn this on without using 
    --jvm-debug
    )? Is the non-explicitness particularly important?
  • e

    enough-analyst-54434

    08/03/2015, 8:31 PM
    it has no effect today
  • e

    enough-analyst-54434

    08/03/2015, 8:32 PM
    My change says it should have an effect and should imply --jvm-debug=True
  • f

    fast-megabyte-33579

    08/03/2015, 8:32 PM
    interesting
  • f

    fast-megabyte-33579

    08/03/2015, 8:32 PM
    is that the 2562 change?
  • e

    enough-analyst-54434

    08/03/2015, 8:32 PM
    yes
  • e

    enough-analyst-54434

    08/03/2015, 8:32 PM
    Reviews welcome
  • e

    enough-analyst-54434

    08/03/2015, 8:32 PM
    The key bit of signal is that the --jvm-debug-port came from the command line
  • e

    enough-analyst-54434

    08/03/2015, 8:33 PM
    WIthout that we couldn't make guesses about what you really mean
  • f

    fast-megabyte-33579

    08/03/2015, 8:33 PM
    true. presumably there's no limit to the 'chain' of impliedness you (could) create
  • f

    fast-megabyte-33579

    08/03/2015, 8:34 PM
    oh I see how it works now
  • r

    rough-minister-58256

    08/03/2015, 8:37 PM
    @happy-kitchen-89482: strange, I get a "E AttributeError: 'TestOptionValues' object has no attribute ‘log_dir’” when ‘—log-dir’ is a registered option on my subsystem
  • r

    rough-minister-58256

    08/03/2015, 8:38 PM
    w/ BaseTest.context()
  • e

    enough-analyst-54434

    08/03/2015, 8:40 PM
    How do you create the subsystem? Via the classmethods?
  • e

    enough-analyst-54434

    08/03/2015, 8:42 PM
    and do you pass 
    for_task_types
    to 
    self.context
    ?
  • r

    rough-minister-58256

    08/03/2015, 8:42 PM
  • e

    enough-analyst-54434

    08/03/2015, 8:43 PM
    yeah - the issue is missing for_task_types
  • e

    enough-analyst-54434

    08/03/2015, 8:44 PM
    The example ivy test you used goes through TaskTest, not BaseTest
  • e

    enough-analyst-54434

    08/03/2015, 8:44 PM
    which handles that part with an override of self.context
  • r

    rough-minister-58256

    08/03/2015, 8:44 PM
    oic
  • e

    enough-analyst-54434

    08/03/2015, 8:45 PM
    If you're going to use the current hack, I think extending the Task base test makes more sense anyway - it justifies the DummyTask a bit better
  • r

    rough-minister-58256

    08/03/2015, 8:48 PM
    will just wait for create_subsystem simple smile
  • e

    enough-analyst-54434

    08/03/2015, 9:14 PM
    It looks like I nuked the coveralls integration with github PRs some time back ... thats fixed and you should be getting coveralls comments on your PRs again now
1...129130131...2420Latest