Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-1052

Add / improve tests and examples for Log package

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: log
    • Labels:
      None
    • Story Points:
      0.5
    • Epic Link:
    • Sprint:
      DB_S14_08
    • Team:
      Data Access and Database

      Attachments

        Issue Links

          Activity

          Hide
          jbecla Jacek Becla added a comment -

          See branch u/jbecla/DM-1052

          git diff --stat  06dddb..d4762e
           doc/mainpage.dox       |   7 +++++++
           include/lsst/log/Log.h |  66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
           tests/cppTest.cc       | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
           3 files changed, 163 insertions(+), 15 deletions(-)

          good enough?

          Show
          jbecla Jacek Becla added a comment - See branch u/jbecla/ DM-1052 git diff --stat 06dddb..d4762e doc/mainpage.dox | 7 +++++++ include/lsst/log/Log.h | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ tests/cppTest.cc | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 163 insertions(+), 15 deletions(-) good enough?
          Hide
          ktl Kian-Tat Lim added a comment -

          Pretty good. The context setting in the context test case is not actually checked; I think you'd need a PatternLayout for that. I think you're also supposed to use BOOST_FIXTURE_TEST_CASE instead of instantiating the fixture yourself, see the docs.

          The macros are getting to be numerous enough that I wonder if we need a macro to generate them...

          Show
          ktl Kian-Tat Lim added a comment - Pretty good. The context setting in the context test case is not actually checked; I think you'd need a PatternLayout for that. I think you're also supposed to use BOOST_FIXTURE_TEST_CASE instead of instantiating the fixture yourself, see the docs . The macros are getting to be numerous enough that I wonder if we need a macro to generate them...
          Hide
          jbecla Jacek Becla added a comment - - edited

          K-T, regarding per-test-case fixtures, I considered using it, but notice that my fixture has some state (temporary output file), and a function check() that relies on it, similar to a generic use case shown in fixtures. Is there a way to reproduce "do something involving f.i" if I switch to per-test-case fixtures? I wasn't able to figure that out.

          Show
          jbecla Jacek Becla added a comment - - edited K-T, regarding per-test-case fixtures, I considered using it, but notice that my fixture has some state (temporary output file), and a function check() that relies on it, similar to a generic use case shown in fixtures . Is there a way to reproduce "do something involving f.i" if I switch to per-test-case fixtures? I wasn't able to figure that out.
          Hide
          ktl Kian-Tat Lim added a comment -

          The example in the document I pointed to has state (the "i" variable). There's no problem accessing check() (without any prefix) if you use BOOST_FIXTURE_TEST_CASE(..., LogFixture).
          There's also this: warning: the use of `tmpnam' is dangerous, better use `mkstemp', but that's hard to work around, and it's probably not worth it for a test.

          Show
          ktl Kian-Tat Lim added a comment - The example in the document I pointed to has state (the "i" variable). There's no problem accessing check() (without any prefix) if you use BOOST_FIXTURE_TEST_CASE(..., LogFixture) . There's also this: warning: the use of `tmpnam' is dangerous, better use `mkstemp' , but that's hard to work around, and it's probably not worth it for a test.
          Hide
          jbecla Jacek Becla added a comment -

          All done, merged to master. Closing

          Show
          jbecla Jacek Becla added a comment - All done, merged to master. Closing

            People

            • Assignee:
              fritzm Fritz Mueller
              Reporter:
              fritzm Fritz Mueller
              Reviewers:
              Kian-Tat Lim
              Watchers:
              Jacek Becla, Kian-Tat Lim
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel