Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.23.0
    • Fix Version/s: None
    • Component/s: test
    • Labels:
      None

      Description

      CommandFormat which is used to parse command lines is following posix conventions of stopping at the first non-argument. There is just one test in this file which placed an option in the middle of the args.

        Issue Links

          Activity

          Hide
          Daryn Sharp added a comment -

          Sorry for the delay, I've been out of town since last Thu. I am changing the parsing behavior to re-add the permute behavior and intend to close this jira as invalid.

          Show
          Daryn Sharp added a comment - Sorry for the delay, I've been out of town since last Thu. I am changing the parsing behavior to re-add the permute behavior and intend to close this jira as invalid.
          Hide
          Todd Lipcon added a comment -

          Hi Daryn. Are you working on fixing this or we should reassign it? Thanks.

          Show
          Todd Lipcon added a comment - Hi Daryn. Are you working on fixing this or we should reassign it? Thanks.
          Hide
          Todd Lipcon added a comment -

          Yea, I guess making it optional makes the most sense. Default should probably be permutation is OK.

          Show
          Todd Lipcon added a comment - Yea, I guess making it optional makes the most sense. Default should probably be permutation is OK.
          Hide
          Daryn Sharp added a comment -

          Yes, I think you are right. I misspoke on the other jira because I didn't remember that a number of commands already used CommandFormat. I suspect people aren't going to be fond of losing the gnu permute behavior... Should I make permute be an option for CommandFormat with a default of on? It would have to be an option since all commands use CommandFormat now, and some like "test" cannot tolerate permute.

          Show
          Daryn Sharp added a comment - Yes, I think you are right. I misspoke on the other jira because I didn't remember that a number of commands already used CommandFormat. I suspect people aren't going to be fond of losing the gnu permute behavior... Should I make permute be an option for CommandFormat with a default of on? It would have to be an option since all commands use CommandFormat now, and some like "test" cannot tolerate permute.
          Hide
          Todd Lipcon added a comment -

          actually, one question before commit: this test case has had this same ordering of arguments since it was introduced. It only started failing once we changed CommandFormat to only allow flags at the beginning. That indicates that the CommandFormat change was a compatibility-breaking change. Thoughts?

          Show
          Todd Lipcon added a comment - actually, one question before commit: this test case has had this same ordering of arguments since it was introduced. It only started failing once we changed CommandFormat to only allow flags at the beginning. That indicates that the CommandFormat change was a compatibility-breaking change. Thoughts?
          Hide
          Todd Lipcon added a comment -

          +1 looks good to me

          Show
          Todd Lipcon added a comment - +1 looks good to me
          Hide
          Daryn Sharp added a comment -

          The findbugs failure is bogus. The TestMRCLI is not related to this patch, it is covered by MAPREDUCE-2565.

          There's a reviewer +1 for the patch, so please considering committing this patch fixes the test it's intended to fix.

          Show
          Daryn Sharp added a comment - The findbugs failure is bogus. The TestMRCLI is not related to this patch, it is covered by MAPREDUCE-2565 . There's a reviewer +1 for the patch, so please considering committing this patch fixes the test it's intended to fix.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481604/MAPREDUCE-2568.patch
          against trunk revision 1132807.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these core unit tests:
          org.apache.hadoop.cli.TestMRCLI

          +1 contrib tests. The patch passed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/353//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/353//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/353//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12481604/MAPREDUCE-2568.patch against trunk revision 1132807. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these core unit tests: org.apache.hadoop.cli.TestMRCLI +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/353//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/353//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-MAPREDUCE-Build/353//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          Good question. Yes, there are positive and negative tests in hadoop common. I question why MR is even testing a class (CommandFormat) that it doesn't "own"...

          Show
          Daryn Sharp added a comment - Good question. Yes, there are positive and negative tests in hadoop common. I question why MR is even testing a class ( CommandFormat ) that it doesn't "own"...
          Hide
          Jeffrey Naisbitt added a comment -

          +1 Looks good to me. Should we have a negative test that verifies the original testcase fails here, or do you already have that tested elsewhere?

          Show
          Jeffrey Naisbitt added a comment - +1 Looks good to me. Should we have a negative test that verifies the original testcase fails here, or do you already have that tested elsewhere?
          Hide
          Daryn Sharp added a comment -

          Move option earlier.

          Show
          Daryn Sharp added a comment - Move option earlier.

            People

            • Assignee:
              Daryn Sharp
              Reporter:
              Daryn Sharp
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development