MRUnit
  1. MRUnit
  2. MRUNIT-46

Tests should use *Driver.new*Driver factory methods for cleaner code

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.9.0
    • Labels:
      None

      Description

      Our tests should use the new MapDriver.new* factory methods as it removes a lot of complexity.

      1. MRUNIT-46-2.patch
        2 kB
        Jim Donofrio
      2. MRUNIT-46.patch
        15 kB
        Jim Donofrio

        Activity

        Hide
        Jim Donofrio added a comment -

        changed all constructor calls to MapDriver, ReduceDriver, and MapReduceDriver in either the mrunit or mrunit.mapreduce packages to use the shortcut newMapDriver, etc calls
        updated instances of these constructor calls in the documentation to the shortcut method
        added shortcut constructor method to include the combiner in the mrunit.MapReduceDriver class

        Show
        Jim Donofrio added a comment - changed all constructor calls to MapDriver, ReduceDriver, and MapReduceDriver in either the mrunit or mrunit.mapreduce packages to use the shortcut newMapDriver, etc calls updated instances of these constructor calls in the documentation to the shortcut method added shortcut constructor method to include the combiner in the mrunit.MapReduceDriver class
        Hide
        Brock Noland added a comment -

        Jim,

        Excellent work! The only comment I have is the method added to o.a.h.mrunit.MapReduceDriver should also be added to o.a.h.mrunit.mapreduce.MapReduceDriver.

        Thoughts?

        Brock

        Show
        Brock Noland added a comment - Jim, Excellent work! The only comment I have is the method added to o.a.h.mrunit.MapReduceDriver should also be added to o.a.h.mrunit.mapreduce.MapReduceDriver. Thoughts? Brock
        Hide
        Jim Donofrio added a comment -

        I would agree but the o.a.h.mrunit.mapreduce.MapReduceDriver currently has no support for combiners

        Show
        Jim Donofrio added a comment - I would agree but the o.a.h.mrunit.mapreduce.MapReduceDriver currently has no support for combiners
        Hide
        Brock Noland added a comment -

        You are correct sir! I will blame it on being tired. I created MRUNIT-67 to address this. Committed in 1293860.

        Thank you for your contribution!

        Show
        Brock Noland added a comment - You are correct sir! I will blame it on being tired. I created MRUNIT-67 to address this. Committed in 1293860. Thank you for your contribution!
        Hide
        Jim Donofrio added a comment -

        Ok that looks good except somehow the patch did not include changing to the shortcut method in the testConfiguration method in the mapreduce.MapDriver and ReduceDriver classes. This patch includes just those changes, maybe it was a svn conflict with one of my other patches?

        Show
        Jim Donofrio added a comment - Ok that looks good except somehow the patch did not include changing to the shortcut method in the testConfiguration method in the mapreduce.MapDriver and ReduceDriver classes. This patch includes just those changes, maybe it was a svn conflict with one of my other patches?
        Hide
        Jim Donofrio added a comment -

        see comment above with new patch MRUNIT-46-2.patch

        Show
        Jim Donofrio added a comment - see comment above with new patch MRUNIT-46 -2.patch
        Hide
        Brock Noland added a comment -

        Yeah I had to do some merging due to the other patches. Committed to trunk in 1293877. Thanks again!

        Show
        Brock Noland added a comment - Yeah I had to do some merging due to the other patches. Committed to trunk in 1293877. Thanks again!
        Hide
        Jim Donofrio added a comment -

        last commit in above post by Brock fixed 2 missing changes

        Show
        Jim Donofrio added a comment - last commit in above post by Brock fixed 2 missing changes

          People

          • Assignee:
            Jim Donofrio
            Reporter:
            Brock Noland
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development