-
Notifications
You must be signed in to change notification settings - Fork 62
Fix for [OODT-970] and [OODT-969] - Tests for Avro File Manager #55
Conversation
…d solve the NPE problem in NettyTransceiver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @IMS94 this is looking very good. Can you consider my comments. thank you
@@ -74,6 +74,16 @@ the License. | |||
<artifactId>aws-java-sdk</artifactId> | |||
<version>1.7.4</version> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.apache.avro</groupId> | |||
<artifactId>avro-ipc</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please make the avro.version
a defined property within the <properties></properties>
element. Then it can be used consistently.
|
||
package org.apache.oodt.cas.filemgr.system; | ||
|
||
import junit.framework.TestCase; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this to org.junit imports and associated anntoations
|
||
public abstract class AbstractFileManagerServerTest extends TestCase { | ||
|
||
private static final Logger LOG = Logger.getLogger(AbstractFileManagerServerTest.class.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we use Slf4j over Log4j... maybe I am wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comments @lewismc! Well, the usage of logging is confusing since at some places it has used java.util.Logging
and in some places it has used slf4j API
with log4j-slf4j implementation
. I personally prefer slf4j
. But in these classes I kept what was being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is confusing. OK, please ignore my comment here then and consider the remaining comments, thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Thank you again. I will fix the others. I will try to make the logging consistent in the future since I believe that logging helps a lot when debugging. ;-)
i'm going to go ahead and commit this. It's sat too long and it's useful in its current form. |
That's fine. Only required addition to this PR is to use JUnit annotations instead of extending TestCase. may be I can do it later. |
awesome thanks @IMS94 OK so build passes on
|
Tests for Avro File Manager and solve the NPE problem in NettyTransceiver when Avro File Manager being called