Skip to content
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

Open
mechjacktv opened this issue Feb 6, 2019 · 10 comments
Open

Create a super interface for each of the event types #251

mechjacktv opened this issue Feb 6, 2019 · 10 comments

Comments

@mechjacktv
Copy link

I was trying to test code that depends on ChannelMessageEvent tonight and there was no easy way to fake ChannelMessageEvent 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 for ChannelMessageEvent 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.

@bendem
Copy link

bendem commented Feb 6, 2019

can't you instantiate the ChannelMessageEvent class? There is no need to extend it since there is no behavior in it.

@mechjacktv
Copy link
Author

mechjacktv commented Feb 6, 2019

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 sendReply or validate that sendReply was called at all (nor should it), but I want to write tests against my code that check for both.

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 ChannelMessageEvent’s dependencies which are irrelevant to my tests.

@mbax
Copy link
Member

mbax commented Feb 7, 2019

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.

@mechjacktv
Copy link
Author

mechjacktv commented Feb 7, 2019

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).

@bendem
Copy link

bendem commented Feb 7, 2019

Can you show a concrete example of what you are trying to do?

@mechjacktv
Copy link
Author

mechjacktv commented Feb 7, 2019

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]);
}

@mbax
Copy link
Member

mbax commented Feb 8, 2019

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");
    }

@mechjacktv
Copy link
Author

mechjacktv commented Feb 8, 2019

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.

@mbax
Copy link
Member

mbax commented Feb 9, 2019

I wonder if changing from provided to compile on checker-qual would make things behave?

[not available for dev stuff until maybe Sunday]

@mechjacktv
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants