-
Notifications
You must be signed in to change notification settings - Fork 625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unit tests for APIPublisherImpl #3869
Conversation
@@ -150,7 +151,7 @@ public static LifecycleState getMockLifecycleStateObject(String lifecycleId) { | |||
LifecycleState lifecycleState = new LifecycleState(); | |||
lifecycleState.setLcName("API_LIFECYCLE"); | |||
lifecycleState.setLifecycleId(lifecycleId); | |||
lifecycleState.setState("PUBLISH"); | |||
lifecycleState.setState("PUBLISHED"); |
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.
Use APIStatus constants
@@ -150,7 +151,7 @@ public static LifecycleState getMockLifecycleStateObject(String lifecycleId) { | |||
LifecycleState lifecycleState = new LifecycleState(); | |||
lifecycleState.setLcName("API_LIFECYCLE"); |
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.
use APIMgtConstants.API_LIFECYCLE
Thanks for the review. Will do changes. |
There are some strings constants in SampleTestObjectCreator.java class (not coming from your PR) . Shall we move them to constants and use existing constants where we can |
Yes, sure. I will move them. |
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.
Shall we avoid adding merge commits? If you rebase the code, then merge commits will not occur.
@@ -20,11 +20,13 @@ | |||
|
|||
package org.wso2.carbon.apimgt.core; | |||
|
|||
|
|||
import org.apache.commons.collections.map.HashedMap; |
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.
Shall we remove the new line?
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.
Will remove it.
Thanks for the review. Will rebase the code. |
No description provided.