-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Create a super interface for each of the event types #251
Comments
can't you instantiate the |
Absolutely and ultimately that’s what I’m doing, but I had to extend it first to get the behavior I needed for testing. This class lives at the edge of my application and I want to write unit tests that validate that I’m calling it with expected arguments. This class doesn’t expose the message that was passed into It’s not about just having an instance to pass in as a dependency but that I have an instance that exposes how my code interacted with it. If I had an interface to code against I could easily do that and not have to worry about providing |
This week, I'm quite busy and I don't have the time to put in the thought for considering this. So, in the mean time: Have you considered mocking it (and if so, why does it not meet your goals)? I've had a lot of fun with mockito for testing things within the library. |
That’s exactly what I want to do. It’s easier to mock an interface than a concrete class (you have more options). Moving from v4 of KICL to v5 also broke Mockito (I think, but am not sure, it’s related to the module support added, which is fine because I prefer mocking with dynamic proxies anyway). |
Can you show a concrete example of what you are trying to do? |
What I have to do today: private static class TestableChannelMessageEvent extends ChannelMessageEvent {
private String replyMessage;
public TestableChannelMessageEvent(Client client, List<ServerMessage> originalMessages,
User user, Channel channel, String message) {
super(client, originalMessages, user, channel, message);
this.replyMessage = null;
}
public void sendReply(String message) {
this.replyMessage = message;
}
public String getReplyMessage() {
return this.replyMessage;
}
}
@Test
public void myTestMethod() {
Client client = (Client) Proxy.newProxyInstance(getClass().getClassLoader(),
new Class[] { Client.class }, (proxy, method, args) -> null);
User client = (User) Proxy.newProxyInstance(getClass().getClassLoader(),
new Class[] { User.class }, (proxy, method, args) -> null);
Channel channel = (Channel) Proxy.newProxyInstance(getClass().getClassLoader(),
new Class[] { Channel.class }, (proxy, method, args) -> null);
TestableChannelMessageEvent event = new TestableChannelMessageEvent(client,
new ArrayList<ServerMessage>(), user, channel, "Arbitrary-1");
SubjectUnderTest subjectUnderTest = new SubjectUnderTest(event);
String testInput = "Arbitrary-2";
subjectUnderTest.doSomething(testInput);
assertEquals(String.format(SubjectUnderTest.MESSAGE_FORMAT, testInput),
event.getReplyMessage());
} What I would like to do: @Test
public void myTestMethod() {
final String[] replyMessage = new String[1];
ChannelMessageEventInterface event = (ChannelMessageEventInterface) Proxy
.newProxyInstance(getClass().getClassLoader(),
new Class[] { ChannelMessageEventInterface.class }, (proxy, method, args) -> {
if("sendReply".equals(method.getName())) {
replyMessage[0] = args[0];
}
return null
});
SubjectUnderTest subjectUnderTest = new SubjectUnderTest(event);
String testInput = "Arbitrary-1";
subjectUnderTest.doSomething(testInput);
assertEquals(String.format(SubjectUnderTest.MESSAGE_FORMAT, testInput), replyMessage[0]);
} |
KICL tests with mockito so it can't be broken for that reason. My guess is that you don't have checker framework in your dependencies, which totally will make mockito fall over (found that out myself while testing tonight!). This code worked fine for me as an example "does my event listener reply": public void test(ChannelMessageEvent event) {
event.sendReply("YAY");
}
@Test
public void testpls() {
ChannelMessageEvent event = Mockito.mock(ChannelMessageEvent.class);
this.test(event);
Mockito.verify(event, Mockito.times(1)).sendReply("YAY");
Mockito.verify(event, Mockito.times(0)).sendReply("BOO");
} |
You are right that I don't have checker framework as a dependency because I'm not using directly. If that's the solution I still submit something about the module implementation is broken. If your answer is that I have to use a tool like Mockito or extend your code to add testing behavior to write tests against your code that's fine. I would just like to not depend on concrete implementations if I don't have to thus the issue being opened. |
I wonder if changing from [not available for dev stuff until maybe Sunday] |
To be clear, make Mockito work was not the goal of my issue. Mockito breaking certainly put things on my radar, but at the end of the day, to use this library requires directly depending on concrete implementations and not interfaces. This will always have an adverse impact on testability. |
I was trying to test code that depends on
ChannelMessageEvent
tonight and there was no easy way to fakeChannelMessageEvent
without extending the concrete class. My code depends on several of the interfaces it implements, but because there is no super interface that pulls them together forChannelMessageEvent
to implement I'm extending.I'd like to see all of the event classes roll up the interfaces they implement into one super interface, one for each of the event types so future testing isn't so hacky.
The text was updated successfully, but these errors were encountered: