Skip to content
Snippets Groups Projects
  1. Mar 01, 2012
    • Nat Goodspeed's avatar
      Make CaptureLog::withMessage() raise tut::failure if not found. · f9048677
      Nat Goodspeed authored
      All known callers were using ensure(! withMessage(...).empty()). Centralize
      that logic. Make failure message report the string being sought and the log
      messages in which it wasn't found.
      In case someone does want to permit the search to fail, add an optional
      'required' parameter, default true.
      Leverage new functionality in llprocess_test.cpp.
      f9048677
    • Nat Goodspeed's avatar
      Break out TestRecorder class as CaptureLog into wrapllerrs.h. · 2596816f
      Nat Goodspeed authored
      Giving more unit tests the ability to capture and examine log output is
      generally useful. Renaming the class just makes it less ambiguous: what's a
      TestRecorder? Something that records tests?
      2596816f
  2. Feb 29, 2012
    • Nat Goodspeed's avatar
      Guarantee LLProcess::Params::postend listener any ReadPipe data. · 3649eda6
      Nat Goodspeed authored
      Previously one might get process-terminated notification but still have to
      wait for the child process's final data to arrive on one or more ReadPipes.
      That required complex consumer timing logic to handle incomplete pending
      ReadPipe data, e.g. a partial last line with no terminating newline. New code
      guarantees that by the time LLProcess sends process-terminated notification,
      all pending pipe data will have been buffered in ReadPipes.
      Document LLProcess::ReadPipe::getPump() notification event; add "eof" key.
      Add LLProcess::ReadPipe::getline() and read() convenience methods.
      Add static LLProcess::getline() and basename() convenience methods, publishing
      logic already present elsewhere.
      Use ReadPipe::getline() and read() in unit tests.
      Add unit test for "eof" event on ReadPipe::getPump().
      Add unit test verifying that final data have been buffered by termination
      notification event.
      3649eda6
  3. Feb 27, 2012
  4. Feb 23, 2012
    • Nat Goodspeed's avatar
      Tighten up LLProcess pipe support, per Richard's code review. · 14ddc647
      Nat Goodspeed authored
      Clarify wording in some of the doc comments; be a bit more explicit about some
      of the parameter fields.
      Make some query methods 'const'.
      Change default LLProcess::ReadPipe::getLimit() value to 0: don't post any
      incoming data with notification event unless caller requests it. But do post
      pertinent FILESLOT in case caller reuses same listener for both stdout and
      stderr.
      Use more idiomatic, readable syntax for accessing LLProcess::Params data.
      14ddc647
  5. Feb 20, 2012
    • Nat Goodspeed's avatar
      Let LLProcess consumer specify desired description for logging. · 999484a6
      Nat Goodspeed authored
      If caller runs (e.g.) a Python script, it's not very helpful to a human log
      reader to keep seeing LLProcess instances logged as /pathname/to/python (pid).
      If caller is aware, the code can at least use the script name as the desc --
      or maybe even a hint as to the script's purpose.
      If caller doesn't explicitly pass a desc, at least shorten to just the
      basename of the executable.
      999484a6
    • Nat Goodspeed's avatar
      Make LLProcess post termination event to specified pump if desired. · 8b5d5f96
      Nat Goodspeed authored
      This way a caller need not spin on isRunning(); we can just listen for the
      requested termination event.
      Post a similar event containing error message if for any reason
      LLProcess::create() failed to launch the child.
      Add unit tests for both cases.
      8b5d5f96
  6. Feb 16, 2012
    • Nat Goodspeed's avatar
      Fix typos in a few LLProcess::ReadPipe::find() unit tests. · f52cf4be
      Nat Goodspeed authored
      The typos didn't make for invalid tests, but they made a few tests redundant
      while leaving other (subtly different) cases untested.
      f52cf4be
    • Nat Goodspeed's avatar
      Fix bug in LLProcess::ReadPipe::peek() substring computation. · a06ba836
      Nat Goodspeed authored
      Add unit tests for peek() with substring args, reimplemented contains(),
      various forms of find().
      (yay unit tests)
      a06ba836
    • Nat Goodspeed's avatar
      4ecf9d6a
    • Nat Goodspeed's avatar
      Add LLProcess::ReadPipe::find() methods, with corresponding npos. · e92c3113
      Nat Goodspeed authored
      If it's useful to have contains() to tell you whether incoming data contains a
      particular substring, and if it's useful for contains() and peek() to accept
      an offset within that data, then it's useful to allow you to get the offset of
      a desired substring within that data. But of course a find() returning offset
      needs something like std::string::npos for "not found"; borrow that
      convention.
      Support both find(const std::string&) and find(char); the latter permits a
      more efficient implementation. In fact, make find(string) recognize a string
      of length 1 and leverage the find(char) implementation.
      Given that, reimplement contains(mumble) as shorthand for find(mumble) != npos.
      Implement find() overloads using std::search() and std::find() on
      boost::asio::streambuf character iterators, rather than copying to std::string
      and then using string search like previous contains() implementation.
      Reimplement WritePipeImpl::tick() and ReadPipeImpl::tick() to write/read
      directly from/to boost::asio::streambuf data, instead of copying to/from a
      temporary flat buffer.
      As long as ReadPipeImpl::tick() keeps successfully filling buffers, keep
      reading. Previous implementation would only handle a long child write over
      successive tick() calls. Stop on read error or when we come up short.
      e92c3113
  7. Feb 15, 2012
    • Nat Goodspeed's avatar
    • Nat Goodspeed's avatar
      Don't be confused by "\r\n" line endings on pipe on Windows. · fc6d70db
      Nat Goodspeed authored
      These are all very well when we just want to dump the output to a log, or
      whatever, but in a unit-test context it matters for comparison.
      fc6d70db
    • Nat Goodspeed's avatar
      Add LLProcess::ReadPipe::size(), peek(), contains(). · 56d93121
      Nat Goodspeed authored
      Also add "len" key to event data on LLProcess::getPump(). If you've used
      setLimit(), event["data"].length() may not reflect the length of the
      accumulated data in the ReadPipe.
      Add unit test with stdin/stdout handshake with child process.
      56d93121
    • Nat Goodspeed's avatar
      Fix llprocess_test.cpp's exception catching for Linux. · 10ab4adc
      Nat Goodspeed authored
      In the course of re-enabling the indra/test tests last year, Log generalized a
      workaround I'd introduced in llsdmessage_test.cpp. In Linux viewer land, a
      test program trying to catch an expected exception can't seem to catch it by
      its specific class (across the libllcommon.so boundary), but must instead
      catch std::runtime_error and validate the typeid().name() string. Log added a
      macro for this idiom in llevents_tut.cpp. Generalize that macro further for
      normal-case processing as well, move it to a header file of its own and use it
      in all known places -- plus the new exception-catching tests in
      llprocess_test.cpp.
      10ab4adc
    • Nat Goodspeed's avatar
    • Nat Goodspeed's avatar
      Preliminary pipe support for LLProcess. · e239cad1
      Nat Goodspeed authored
      Add LLProcess::FileParam to specify how to construct each child's standard
      file slot, with lots of comments about features designed but not yet
      implemented. The point is to design it with enough flexibility to be able to
      extend to foreseeable use cases.
      Add LLProcess::Params::files to collect up to 3 FileParam items. Naturally
      this extends the accepted LLSD syntax as well.
      Implement type="" (child inherits parent file descriptor) and "pipe" (parent
      constructs anonymous pipe to pass to child).
      Add LLProcess::FILESLOT enum, plus methods:
      getReadPipe(FILESLOT), getOptReadPipe(FILESLOT)
      getWritePipe(), getOptWritePipe()
      getPipeName(FILESLOT): placeholder implementation for now
      Add LLProcess::ReadPipe and WritePipe classes, as returned by get*Pipe().
      WritePipe supports get_ostream() method for streaming to child stdin.
      ReadPipe supports get_istream() method for reading from child stdout/stderr.
      It also provides getPump() returning LLEventPump& so interested parties can
      listen for arrival of new data on the aforementioned std::istream.
      For "pipe" slots, instantiate appropriate *Pipe class.
      ReadPipe and WritePipe classes are pure virtual bases for ReadPipeImpl and
      WritePipeImpl, respectively: all implementation data are hidden in the latter
      classes, visible only in llprocess.cpp. In fact each *PipeImpl class registers
      itself for "mainloop" ticks, attempting nonblocking I/O to the underlying
      apr_file_t on each tick. Data are buffered in a boost::asio::streambuf, which
      bridges between std::[io]stream and the APR I/O calls.
      Sanity-test ReadPipeImpl by using a pipe to absorb the Python "SyntaxError"
      output from the successful syntax_error test, rather than alarming the user.
      Add first few unit tests for validating FileParam. More tests coming!
      e239cad1
  8. Feb 13, 2012
    • Nat Goodspeed's avatar
      Use per-frame ticks on "mainloop" LLEventPump to update LLProcess. · aae61392
      Nat Goodspeed authored
      When we reimplemented LLProcess on APR, necessitating APR's funny callback
      mechanism to sense child-process status, every isRunning() or getStatus() call
      called the APR poll function that calls ALL registered LLProcess callbacks. In
      other words, every time any consumer called any LLProcess::isRunning() method,
      all LLProcess callbacks were redundantly fired. Change that so that the single
      APR poll function is called once per frame, courtesy of the "mainloop"
      LLEventPump. Once per viewer frame should be well within the realtime duration
      in which it's reasonable to expect child-process status to change.
      In effect, this changes LLProcess's public API to introduce a dependency on
      "mainloop" ticks. Add such ticks to llprocess_test.cpp as well.
      aae61392
    • Nat Goodspeed's avatar
      d4f887e4
  9. Feb 07, 2012
    • Nat Goodspeed's avatar
      Use os.path.normcase(os.path.normpath()) when comparing directories. · 32e11494
      Nat Goodspeed authored
      Once again we've been bitten by comparison failure between "c:\somepath" and
      "C:\somepath". Normalize paths in both Python helper scripts to make that
      comparison more robust.
      32e11494
    • Nat Goodspeed's avatar
      Convert LLProcess implementation from platform-specific to using APR. · aafb03b2
      Nat Goodspeed authored
      Include logic to engage Linden apr_procattr_autokill_set() extension: on
      Windows, magic CreateProcess() flag must be pushed down into apr_proc_create()
      level. When using an APR package without that extension, present
      implementation should lock (e.g.) SLVoice.exe lifespan to viewer's on Windows
      XP but probably won't on Windows 7: need magic flag on CreateProcess().
      Using APR child-termination callback requires us to define state (e.g.
      LLProcess::RUNNING). Take the opportunity to present Status, capturing state
      and (if terminated) rc or signal number; but since most of the time all caller
      really wants is to log the outcome, also present status string, encapsulating
      logic to examine state and describe exited-with-rc vs. killed-by-signal.
      New Status logic may report clearer results in the case of a Windows child
      process killed by exception.
      Clarify that static LLProcess::isRunning(handle) overload is only for use when
      the original LLProcess object has been destroyed: really only for unit tests.
      We necessarily retain our original platform-specific implementations for just
      that one method. (Nonstatic isRunning() no longer calls static method.)
      Clarify log output from llprocess_test.cpp in a couple places.
      aafb03b2
  10. Jan 30, 2012
    • Nat Goodspeed's avatar
      Expose 'handle' as well as 'id' on LLProcess objects. · 85581eef
      Nat Goodspeed authored
      On Posix, these and the corresponding getProcessID()/getProcessHandle()
      accessors produce the same pid_t value; but on Windows, it's useful to
      distinguish an int-like 'id' useful to human log readers versus an opaque
      'handle' for passing to platform-specific API functions. So make the
      distinction in a platform-independent way.
      85581eef
  11. Jan 21, 2012
    • Nat Goodspeed's avatar
      Convert LLProcess consumers from LLSD to LLProcess::Params block. · 47d94757
      Nat Goodspeed authored
      Using a Params block gives compile-time checking against attribute typos. One
      might inadvertently set myLLSD["autofill"] = false and only discover it when
      things behave strangely at runtime; but trying to set myParams.autofill will
      produce a compile error.
      However, it's excellent that the same LLProcess::create() method can accept
      either LLProcess::Params or a properly-constructed LLSD block.
      47d94757
  12. Jan 20, 2012
    • Nat Goodspeed's avatar
      Per Richard, replace LLProcessLauncher with LLProcess. · f0dbb878
      Nat Goodspeed authored
      LLProcessLauncher had the somewhat fuzzy mandate of (1) accumulating
      parameters with which to launch a child process and (2) sometimes tracking the
      lifespan of the ensuing child process. But a valid LLProcessLauncher object
      might or might not have ever been associated with an actual child process.
      LLProcess specifically tracks a child process. In effect, it's a fairly thin
      wrapper around a process HANDLE (on Windows) or pid_t (elsewhere), with
      lifespan management thrown in. A static LLProcess::create() method launches a
      new child; create() accepts an LLSD bundle with child parameters. So building
      up a parameter bundle is deferred to LLSD rather than conflated with the
      process management object.
      Reconcile all known LLProcessLauncher consumers in the viewer code base,
      notably the class unit tests.
      f0dbb878
  13. Jan 18, 2012
  14. Jan 17, 2012
    • Nat Goodspeed's avatar
    • Nat Goodspeed's avatar
      Refactor llprocesslauncher_test.cpp for better code reuse. · 2ae9f921
      Nat Goodspeed authored
      Instead of free python() and python_out() functions containing a local
      temporary LLProcessLauncher instance, with a 'tweak' callback param to
      "do stuff" to that inaccessible object, change to a PythonProcessLauncher
      class that sets up a (public) LLProcessLauncher member, then allows you to
      run() or run() and then readfile() the output. Now you can construct an
      instance and tweak to your heart's content -- without funky callback syntax --
      before running the script.
      Move all such helpers from TUT fixture struct to namespace scope. While
      fixture-struct methods can freely call one another, introducing a nested class
      gets awkward: constructor must explicitly require and bind a fixture-struct
      pointer or reference. Namespace scope solves this.
      (Truthfully, I only put them in the fixture struct originally because I
      thought it necessary for calling ensure() et al. But ensure() and friends are
      free functions; need only qualify them with tut:: namespace.)
      2ae9f921
    • Nat Goodspeed's avatar
      Add first couple of LLProcessLauncher tests. · 74fbd318
      Nat Goodspeed authored
      Run INTEGRATION_TEST_llprocesslauncher using setpython.py so we can find the
      Python interpreter of interest.
      Introduce python() function to run a Python script specified using
      NamedTempFile conventions.
      Introduce a convention by which we can read output from a Python script using
      only the limited pre-January-2012 LLProcessLauncher API. Introduce
      python_out() function to leverage that convention.
      Exercise a couple of LLProcessLauncher methods using all the above.
      74fbd318
  15. Jan 13, 2012
    • Nat Goodspeed's avatar
      Extract APR and temp-fixture-file helper code to indra/test. · b6a08ad0
      Nat Goodspeed authored
      Specifically:
      Introduce ManageAPR class in indra/test/manageapr.h. This is useful for a
      simple test program without lots of static constructors.
      Extract NamedTempFile from llsdserialize_test.cpp to indra/test/
      namedtempfile.h. Refactor to use APR file operations rather than platform-
      dependent APIs.
      Use NamedTempFile for llprocesslauncher_test.cpp.
      b6a08ad0
  16. Dec 23, 2011
  17. Dec 22, 2011
    • Nat Goodspeed's avatar
      Comment out lookup table used only by commented-out code. · 29273ffb
      Nat Goodspeed authored
      Otherwise the unreferenced declaration causes a fatal warning.
      29273ffb
    • Nat Goodspeed's avatar
      Never call apr_proc_wait() inside child_status_callback(). · 6ccba881
      Nat Goodspeed authored
      Quiet the temporary child_status_callback() output.
      Add a bit of diagnostic info if apr_proc_wait() returns anything but
      APR_CHILD_DONE.
      6ccba881
    • Nat Goodspeed's avatar
      Add child_status_callback() function and arrange to call periodically. · 39c3efbd
      Nat Goodspeed authored
      At least on OS X 10.7, a call to apr_proc_wait(APR_NOWAIT) in fact seems to
      block the caller. So instead of polling apr_proc_wait(), use APR callback
      mechanism (apr_proc_other_child_register() et al.) and poll that using
      apr_proc_other_child_refresh_all().
      Evidently this polls the underlying system waitpid(), but the internal call
      seems to better support nonblocking. On arrival in the
      child_status_callback(APR_OC_REASON_DEATH) call, though, apr_proc_wait()
      produces ECHILD: the child process in question has already been reaped.
      The OS-encoded wait() status does get passed to the callback, but then we have
      to use OS-dependent macros to tease apart voluntary termination vs. killed by
      signal... a bit of a hole in APR's abstraction layer.
      Wrap ensure_equals() calls with a macro to explain which comparison failed.
      39c3efbd
  18. Dec 21, 2011
Loading