Minor Issues With Unit Tests

[tom-ohare]

Hi,

Thanks for the new release.

While playing around with it I noticed a few quirks, mainly in regard to the tests.

1. The tests don't run during the normal maven build because the test class ATestCases doesn't match the maven defaults (Test*, *Test or *TestCase). Maybe that was intentional on your part. If not, the quickest fix is just to change the class name to ATestCase (though that name is a little misleading).
2. When I first tried to run the tests I had some XML namespace-related errors referring the activemq.xml, specifically the broker tag. In the end, I found that including activemq-all as a test dependency in the pom instead of just activemq-broker made them go away. I'm still not entirely sure what was happening.
3. In the broker tag in activemq.xml, I guess you meant to specify dataDirectory="target/amqdata" and not dataDirectory="taget/amqdata"
4. You might choose to set a property for the encoding in the pom (of course, it's up to you):

    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

The good news is that from the general, operational testing I've been able to do everything works!

[northlander]

Okay, I must admit I've done the testing manually to a large extent. Pretty recently I've started out with some unit tests but, as you point out, they are not fully covering the code. The problem is that it requires pretty decent amount of test code to cover this limited code base.

Thanks for your input, I agree with your input. When time comes, there will be extended unit tests and correction of the file names.

I did a new release (1.1.0) with the fixes of your last discovery and AMQP support (not well tested though).

[northlander]

Btw, there is a nice feature called "Pull request" at GitHub which is great if you cannot wait for me writing unit tests ;-)

[tom-ohare]

I agree that it would take a lot of tests to cover all the possible combinations.

If you like, I can try to write some tests on copy/move (good for me as our main intended use of 'a' is to selectively process messages that have ended up in various dead-letter queues).

Is it OK to rename your existing test class or would you rather I worked from a new test class?

[northlander]

As you please! :)

[northlander]

Added a base test class with some basic tests including copy and move. Fixed some minor maven and AMQ config issues as well. The tests now starts with mvn test or mvn install.

The test suit can be run using various AMQ configs and (jms-compliant) transports. Two test runners exists, AMQP and OpenWire.

Added Travis CI for automatic builds and a marker indicating passing or failing test cases.

I still admit more tests should be done for full coverage, but I guess this is enough to close this issue.

[tom-ohare]

That was quick!

Yes, this is fine for me - more than I was thinking of, actually.

I did discover one tiny issue when I happened to run the tests on a Windows box. The testGetCount method fails because of the line separator. Ill issue a pull request for the fix.

If I have time during the week, I'll try to add tests for moving/copying with selectors (which is one of the things we want to use the tool for).

I also like the Travis marker.

Great work!

[northlander]

Thanks. Haven't done much move/copies with selectors, so that could be worth testing as well.

[northlander]

Added a few tests for copy/move with selectors. Also updated docs to point out that move + find (search in content) does not work.

[tom-ohare]

Great work! I think that's a pretty convincing set of tests.

Just one question: I notice that the testMoveSelector and testCopySelector methods of the BaseTest class set up the command line via:
final String cmdLine = getConnectCommand() + "-" + CMD_MOVE_QUEUE + " SOURCE.QUEUE -s identity='theOne' TARGET.QUEUE";
final String cmdLine = getConnectCommand() + "-" + CMD_COPY_QUEUE + " SOURCE.QUEUE -s identity='theOne' TARGET.QUEUE";

But I would have expected something like:
final String cmdLine = getConnectCommand() + "-s identity='theOne' " + "-" + CMD_MOVE_QUEUE + " SOURCE.QUEUE TARGET.QUEUE";
final String cmdLine = getConnectCommand() + "-s identity='theOne' " + "-" + CMD_COPY_QUEUE + " SOURCE.QUEUE TARGET.QUEUE";

I have a suspicion the existing command line only works because there are no blanks in the selector but wouldn't work for a selector like: identity like 'theOne%'

Have I misunderstood something?

[northlander]

Well.. the test cases simply simulates what a real terminal/command prompt like bash or cmd.exe would do - split on space. However, I added a space and made the split on "space when not inside a quoted string" to come closer to what bash (or similar) would do. Anyway, it works with spaces as long as you quote the selector. Although not as you described but like: -s "identity='the One'". It's the same thing when you pass a property with -H. You need to quote if there are spaces in the text. -H "foo=bar"

[tom-ohare]

OK, my bad.