Uploaded image for project: 'Apache Gora'
  1. Apache Gora
  2. GORA-229

Use @Ignore for unimplemented functionality to identify absent tests

Details

    • Bug
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 0.3
    • 0.4
    • testing

    Description

      We support JUnit 4.10, but actually use it!!!
      All imports should be org.junit instead of junit.framework.*
      We also do not make use of the very handy @Ignore annotation provided by JUnit 4.X... we should.

      Attachments

        1. GORA-229.patch
          100 kB
          Apostolos Giannakidis
        2. GORA-229.patch
          101 kB
          Apostolos Giannakidis

        Activity

          Attached are paches for the gora-accumulo, gora-cassandra, gora-core and gora-hbase modules. I used import static org.junit.Assert.*; as it is recommended by junit. http://junit.sourceforge.net/javadoc/org/junit/Assert.html

          I have also added the @Ignore annotation in the tests that had empty body.

          ap.giannakidis Apostolos Giannakidis added a comment - Attached are paches for the gora-accumulo, gora-cassandra, gora-core and gora-hbase modules. I used import static org.junit.Assert.*; as it is recommended by junit. http://junit.sourceforge.net/javadoc/org/junit/Assert.html I have also added the @Ignore annotation in the tests that had empty body.

          Some JUnit test case classes, such as the TestHBaseStore.java have a main method. Is this method necessary? I think that having main methods in the JUnit test case classes is not a best practice.

          ap.giannakidis Apostolos Giannakidis added a comment - Some JUnit test case classes, such as the TestHBaseStore.java have a main method. Is this method necessary? I think that having main methods in the JUnit test case classes is not a best practice.

          Yeah some people recommend using static import for improving readability. IMHO we shouldn't use them outside this JUnit stuff because it might get trickier later on when handling those in different parts of our code. But I think if we are going to use this static imports, we should import specific members so we can find out more easily where the imported methods are coming from.

          renato2099 Renato Javier MarroquĂ­n Mogrovejo added a comment - Yeah some people recommend using static import for improving readability. IMHO we shouldn't use them outside this JUnit stuff because it might get trickier later on when handling those in different parts of our code. But I think if we are going to use this static imports, we should import specific members so we can find out more easily where the imported methods are coming from.

          Hi Apos,
          sorry to be a pain but is it possible for you to roll the patches in to one patch which we can apply to trunk?
          Thank you

          lewismc Lewis John McGibbney added a comment - Hi Apos, sorry to be a pain but is it possible for you to roll the patches in to one patch which we can apply to trunk? Thank you
          ap.giannakidis Apostolos Giannakidis added a comment - - edited

          I made the changes according to Renato's recommendation to use explicit import statements. I also created a single patch file for all the modules, which I uploaded.

          The following files have been changed:
          org.apache.gora.accumulo.store.AccumuloStoreTest
          org.apache.gora.accumulo.store.PartitionTest
          org.apache.gora.accumulo.util.HexEncoderTest
          org.apache.gora.accumulo.util.SignedBinaryEncoderTest
          org.apache.gora.avro.TestPersistentDatumReader
          org.apache.gora.avro.store.TestAvroStore
          org.apache.gora.mapreduce.MapReduceTestUtils
          org.apache.gora.mapreduce.TestGoraInputFormat
          org.apache.gora.mapreduce.TestGoraInputSplit
          org.apache.gora.mapreduce.TestPersistentSerialization
          org.apache.gora.persistency.impl.TestPersistentBase
          org.apache.gora.persistency.impl.TestStateManagerImpl
          org.apache.gora.persistency.TestListGenericArray
          org.apache.gora.query.impl.TestQueryBase
          org.apache.gora.store.DataStoreTestBase
          org.apache.gora.store.DataStoreTestUtil
          org.apache.gora.store.TestDataStoreFactory
          org.apache.gora.store.WSDataStoreTestBase
          org.apache.gora.util.TestIOUtils
          org.apache.gora.util.TestWritableUtils
          org.apache.gora.dynamodb.TestDynamoDBStore
          org.apache.gora.hbase.store.TestHBaseStore
          org.apache.gora.hbase.util.TestHBaseByteInterface

          Cheers,
          Apos

          ap.giannakidis Apostolos Giannakidis added a comment - - edited I made the changes according to Renato's recommendation to use explicit import statements. I also created a single patch file for all the modules, which I uploaded. The following files have been changed: org.apache.gora.accumulo.store.AccumuloStoreTest org.apache.gora.accumulo.store.PartitionTest org.apache.gora.accumulo.util.HexEncoderTest org.apache.gora.accumulo.util.SignedBinaryEncoderTest org.apache.gora.avro.TestPersistentDatumReader org.apache.gora.avro.store.TestAvroStore org.apache.gora.mapreduce.MapReduceTestUtils org.apache.gora.mapreduce.TestGoraInputFormat org.apache.gora.mapreduce.TestGoraInputSplit org.apache.gora.mapreduce.TestPersistentSerialization org.apache.gora.persistency.impl.TestPersistentBase org.apache.gora.persistency.impl.TestStateManagerImpl org.apache.gora.persistency.TestListGenericArray org.apache.gora.query.impl.TestQueryBase org.apache.gora.store.DataStoreTestBase org.apache.gora.store.DataStoreTestUtil org.apache.gora.store.TestDataStoreFactory org.apache.gora.store.WSDataStoreTestBase org.apache.gora.util.TestIOUtils org.apache.gora.util.TestWritableUtils org.apache.gora.dynamodb.TestDynamoDBStore org.apache.gora.hbase.store.TestHBaseStore org.apache.gora.hbase.util.TestHBaseByteInterface Cheers, Apos

          Hi Apos. Thanks v much for the patch. Two things,

          • All gora-cassandra stuff seems to be absent in the new patch.
          • There ar three methods in TestHBaseStore which are overridden which should also be annotated with @Ignore.

          Just a quick note, JUnit API also enables us to write the following

          @Ignore("not ready yet") 
          @Test public void something() { ...
          

          This means that if there is any reasoning or comments accompanying the code this can be shifted in to the @Ignore annotation input.

          It is entirely up to you Apos, but it is very useful for other developers if you are able to leave patches as they progress here in Jira as oppose to removing them.

          Thank you so much Apos.

          lewismc Lewis John McGibbney added a comment - Hi Apos. Thanks v much for the patch. Two things, All gora-cassandra stuff seems to be absent in the new patch. There ar three methods in TestHBaseStore which are overridden which should also be annotated with @Ignore. Just a quick note, JUnit API also enables us to write the following @Ignore( "not ready yet" ) @Test public void something() { ... This means that if there is any reasoning or comments accompanying the code this can be shifted in to the @Ignore annotation input. It is entirely up to you Apos, but it is very useful for other developers if you are able to leave patches as they progress here in Jira as oppose to removing them. Thank you so much Apos.

          Patch reuploaded according to Lewi's suggestions.

          Thank you Renato and Lewis for your advise.

          Your feedback on this new patch is very welcome!

          Cheers,
          Apos

          ap.giannakidis Apostolos Giannakidis added a comment - Patch reuploaded according to Lewi's suggestions. Thank you Renato and Lewis for your advise. Your feedback on this new patch is very welcome! Cheers, Apos

          Hi Apos, sorry for taking a while to get around to this.
          I cannot apply the patch to SVN. Right now (until folks wish to push the migration to Git for Gora) we use SVN for all version control.
          You can generate your SVN compatible patch as follows

          git diff --no-prefix trunk > $ISSUE_NUMBER.patch
          

          I also notice that the formatting in this patch seems to be all off. It looks like in many instances line indents are replaced by tabs instead of two spaces.
          Thanks Apos. Great work, this is turning out to be a large patch indeed!!!

          lewismc Lewis John McGibbney added a comment - Hi Apos, sorry for taking a while to get around to this. I cannot apply the patch to SVN. Right now (until folks wish to push the migration to Git for Gora) we use SVN for all version control. You can generate your SVN compatible patch as follows git diff --no-prefix trunk > $ISSUE_NUMBER.patch I also notice that the formatting in this patch seems to be all off. It looks like in many instances line indents are replaced by tabs instead of two spaces. Thanks Apos. Great work, this is turning out to be a large patch indeed!!!

          Hey Lewis,

          Yes, it seems that I mistakenly produced the patch using git. I reuploaded the patch generated by svn diff this time.
          Please let me know if this time is correct and if the indentation is as expected.

          Thanks,
          Apos

          ap.giannakidis Apostolos Giannakidis added a comment - Hey Lewis, Yes, it seems that I mistakenly produced the patch using git. I reuploaded the patch generated by svn diff this time. Please let me know if this time is correct and if the indentation is as expected. Thanks, Apos

          Committed @revision 1505236 in trunk.
          Thanks Apos for this patch. Eventually we got there

          lewismc Lewis John McGibbney added a comment - Committed @revision 1505236 in trunk. Thanks Apos for this patch. Eventually we got there

          People

            ap.giannakidis Apostolos Giannakidis
            lewismc Lewis John McGibbney
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: