This week's giveaway is in the EJB and other Java EE Technologies forum. We're giving away four copies of EJB 3 in Action and have Debu Panda, Reza Rahman, Ryan Cuprak, and Michael Remijan on-line! See this thread for details.
Originally posted by Kamal Ahmed: any ideas on how can I test more using regex.
I'm afraid I don't understand what you mean by "test more using regex". Could you be more specific?
In any case, a couple of notes:
1) That's a constructor, not a method.
2) The test method you've posted is actually testing three different inputs to the constructor -- I'd rather see those as separate test methods with descriptive names:
Note how I've had to name the tests with "...AndDefault", "...AndTrue" and "...AndFalse" because I had no idea based on your test code what the purpose of that boolean argument is! When you write your tests with the next guy in mind, you'll eventually thank yourself for being so thoughtful. Readability is crucial in test code just as it is in production code.
Furthermore, you're doing a lot of similar things in each of the methods which is a clear hint for doing some refactoring to remove duplication...
Thank you very much for your valuable input. I will make sure that the problem is more readable and understandable next time.
1. You are correct, this is a constructor 2. By using regex, i meant to test various regex's as inputs, and these would test the names of Loggers 3. This constructor is based on log4j , and this is an API i am testing based on log4j 4. I am not entirely sure what the actual objective is, here is some clue:
Once you get the logger, target, you should be able to just target.error( �here�s my test string� ); The logger will create the logging event and forward it to your appender. The kind of thing I had in mind was to define two appenders that use this filter. One appender calls the filter and uses JUnit assertTrue on the result � expecting the filter to pass the message. The other appender will be exactly the same except it will use assertFalse � expecting the filter to reject the message. You can use the appenders with a couple of string arrays containing variations on logger names. An array of matchers would have logger names that should match your regex. An array of non-matchers would have logger names that shouldn�t. Then you can loop through each array, get a logger and log a message. Attach the first appender to the root logger and then log the matching messages. If the filter handles all the matches properly, all the assertTrue()s will be satisfied. Then remove that appender and attach the other and log the non-matching messages. This type of arrangement ought to support several different tests with different filter configurations. You may be able to do this with one appender, and have it use either assertTrue or assertFalse, depending on a configuration flag.
Thanks, -Kamal. [ January 25, 2006: Message edited by: Kamal Ahmed ]
Joined: Jan 23, 2002
I still don't get it. If you're testing the filter, why create appenders? Why not just invoke the filter directly? Maybe I'd understand if I saw what's inside the WmLoggerFilter.
Joined: Feb 15, 2005
You could be right, i wish i could give you more information on what the requirement actually is, but i am learning about it as i go along. Anyway, here is WmLoggerFilter
Joined: Jan 23, 2002
Ok. Now I see. You have an implementation of Log4J's abstract Filter class that you'd like to unit test.
Firstly, when looking at the interface specified by the Filter class, we can determine that the essential behavior (and thus the behavior we should probably focus on) is the "decide()" method.
The setTargetString() and setAcceptMatch() methods aren't even used outside of the class (only from the constructor) so perhaps they should be made private? At the very least, the getters for those attributes are unnecessary since there's usually little sense in testing that "the getter returns the correct value after the setter has been called". In some cases, perhaps, but in the vast majority of cases it doesn't.
I would just delete the activateOptions() method since it's doing nothing except invoking the parent class's implementation. I presume you left it there as a reminder of such method's existence?
Oh, and I'd also recommend not making methods final unless you have a good reason. Final methods are a major pain in the ass considering testability--because they cannot be overridden.
After a couple of quick refactorings, this is what I'd come up with for the production code (without knowing anything about implementing Log4J Filters, e.g. whether those public setters are needed in order to configure the logger from a configuration file or something):