Initial test dedicated for the DefaultUnknownHandlerManager #205
Conversation
|
I'm not seeing the relationship between what I have changed and the error given by Jenkins. Can someone explain please to fix it? |
| @@ -0,0 +1,69 @@ | |||
| package com.opensymphony.xwork2; | |||
There was a problem hiding this comment.
Please copy and paste the LICENCE text from a neighbor test .java file to beginning of this file to pass Jenkins build :)
There was a problem hiding this comment.
thanks, this is clear and fixed.
| /** | ||
| * Partial test to the DefaultUnknownHandlerManager to understand the relationship between Manager and Handlers. | ||
| * | ||
| * @author Ziyad Alsaeed |
There was a problem hiding this comment.
Apache doesn't accept author tag anymore. Instead see here if you're interested to contribute :)
| /** | ||
| * Relationshsip when UnknownAction method is called. | ||
| * | ||
| * @author Ziyad Alsaeed |
| /** | ||
| * Relationship when UnknownActionMethod method called. | ||
| * | ||
| * @author Ziyad Alsaeed |
|
|
||
| ActionConfig.Builder actionConfigBuilder = new ActionConfig.Builder( "com", "someAction", "someClass"); | ||
| ActionConfig actionConfig = actionConfigBuilder.build(); | ||
| SomeUnknownHandler someUnknownHandler = new SomeUnknownHandler(); |
There was a problem hiding this comment.
It's a good practice to initialise such things in setUp method.
There was a problem hiding this comment.
you are right, fixed.
|
Looks good to me, let @yasserzamani to pull the trigger :) |
|
I see there is already a dedicated test for |
|
This is a tricky one. I'm not an expert in struts but this is slightly different from the |
| * Partial test to the DefaultUnknownHandlerManager to understand the relationship between Manager and Handlers. | ||
| * | ||
| */ | ||
| public class DefaultUnknownHandlerManagerTest extends XWorkTestCase { |
There was a problem hiding this comment.
I tested extends TestCase and works. It seems you can downgrade to TestCase to save performance.
There was a problem hiding this comment.
I was following other test cases here. I will change it and push it if tests pass.
| } | ||
|
|
||
| @Override | ||
| protected void tearDown() throws Exception { |
There was a problem hiding this comment.
It seems you can downgrade to TestCase to save performance and delete this.
|
|
||
| String result = null; | ||
|
|
||
| for (int i = 0; i < 10 ; i++) { |
There was a problem hiding this comment.
Why 10 times? I'm just curious :)
There was a problem hiding this comment.
It is an arbitrary number. I wanted to see the difference this will make on what I'm developing.
Hm.. but coverall says it's almost fully covered in last master build before this pr at here. I also reviewed that previous test case and saw it already has tested
Hm.. you're right. It seems it was there previously and moved here with no moved tests :) |
|
I think wasn't clear when I said "were not touched thoroughly". What I meant is not related to if a line is touched or not (the general definition of test coverage). After all as I mentioned before, the class
But still there is no test class with the name |
|
No I also agree and your class name is good. And you're right, let keep both files as is and as @lukaszlenart Thanks 👍 |
This is a very trivial test dedicated for the DefaultUnknownHandlerManager.
I'm doing some research that includes Apache Struts as an example. Most of the classes I'm interested in have sufficient tests. However, even though DefaultUnknownHandlerManager is tested through other tests created for other classes, those tests are not sufficient enough for my purpose. I'm pushing the given test for DefaultUnknownHandlerManager to confirm that it is acceptable by you.