Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1500

TestOfflineImageViewer failing on trunk

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: test, tools
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Testcase: testOIV took 22.679 sec
      FAILED
      Failed reading valid file: No image processor to read version -26 is available.
      junit.framework.AssertionFailedError: Failed reading valid file: No image processor to read version -26 is available.
      at org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer.outputOfLSVisitor(TestOfflineImageViewer.java:171)
      at org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer.testOIV(TestOfflineImageViewer.java:86)

      1. hdfs-1500.txt
        6 kB
        Todd Lipcon

        Issue Links

          Activity

          Hide
          Todd Lipcon added a comment -

          Looks like we just forgot to update OIV's loadable image versions in HDFS-903

          Show
          Todd Lipcon added a comment - Looks like we just forgot to update OIV's loadable image versions in HDFS-903
          Hide
          Hairong Kuang added a comment -

          +1. Thanks Todd! I did a lousy checkin of HDFS-903.

          Show
          Hairong Kuang added a comment - +1. Thanks Todd! I did a lousy checkin of HDFS-903 .
          Hide
          Hairong Kuang added a comment -

          The patch is trivial and the failed test passed with the patch:
          [junit] Running org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer
          [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 7.252 sec

          Show
          Hairong Kuang added a comment - The patch is trivial and the failed test passed with the patch: [junit] Running org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 7.252 sec
          Hide
          Hairong Kuang added a comment -

          I've just committed this!

          Show
          Hairong Kuang added a comment - I've just committed this!
          Hide
          Jakob Homan added a comment -

          Why was the one-line needed to correct the test failure not the entirety of this patch? The extra changes made to the tests were out of the scope of the JIRA, which got opened and resolved in an hour. By moving the fail statements out of the individual sub-tests, it is now more difficult to see which sub-test failed. Had this been called out as being part of this patch, I would have raised concerns about this change.

          Show
          Jakob Homan added a comment - Why was the one-line needed to correct the test failure not the entirety of this patch? The extra changes made to the tests were out of the scope of the JIRA, which got opened and resolved in an hour. By moving the fail statements out of the individual sub-tests, it is now more difficult to see which sub-test failed. Had this been called out as being part of this patch, I would have raised concerns about this change.
          Hide
          Todd Lipcon added a comment -

          Apologies about not calling that out, Jakob.

          The fail() statements were actually counterproductive as I was debugging the test. The issue is that appending exception.toString() ends up obscuring the stack trace of any failure. Without the catch/fail, you can still always see which function the exception came from, so you don't lose anything.

          Show
          Todd Lipcon added a comment - Apologies about not calling that out, Jakob. The fail() statements were actually counterproductive as I was debugging the test. The issue is that appending exception.toString() ends up obscuring the stack trace of any failure. Without the catch/fail, you can still always see which function the exception came from, so you don't lose anything.
          Hide
          Jakob Homan added a comment -

          That's reasonable. Since patches to fix failing tests are generally committed quickly, without much time for community input, in the future it will be best to save improvements to those tests for subsequent JIRAs, where the community can get a chance to see the changes. This applies for even relatively small changes.

          Show
          Jakob Homan added a comment - That's reasonable. Since patches to fix failing tests are generally committed quickly, without much time for community input, in the future it will be best to save improvements to those tests for subsequent JIRAs, where the community can get a chance to see the changes. This applies for even relatively small changes.
          Hide
          Hairong Kuang added a comment -

          > The fail() statements were actually counterproductive as I was debugging the test.

          I had the same problem when I debugged TestOfflineImage while working on some other image-related jiras. So I thought that was good change and committed it without raising any question.

          Show
          Hairong Kuang added a comment - > The fail() statements were actually counterproductive as I was debugging the test. I had the same problem when I debugged TestOfflineImage while working on some other image-related jiras. So I thought that was good change and committed it without raising any question.

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development