Make artemis friendly to unit tests

[ogregoire]

My business logic is in my systems. This is what I get to test the most when I'm using this framework. But it's rather hard to actually test using this framework. First there's no guidelines on how to do it. Second, the API is absolutely hostile to unit-tests at the moment (I mean hostile, not impossible).

Given the following system,

@Wire
public class MySystem extends PassiveSystem {
  ComponentMapper<Comp> compMapper;
  ComponentMapper<Other> otherMapper;
  public void doSomething(Entity a, Entity b) {
    Comp compA = compMapper.getSafe(a);
    Other otherA = otherMapper.getSafe(a);
    Comp compB = compMapper.getSafe(b);
    Other otherB = otherMapper.getSafe(b);
    // business logic
  }
}

Typically, here's what a test should look like:

public class MySystemTest {
  MySystem instance = new MySystem();
  FakeComponentMapper<Comp> compMapper = new FakeComponentMapper<>();
  FakeComponentMapper<Other> otherMapper = new FakeComponetnMapper<>();
  @Before public void setUp() {
    instance.compMapper = compMapper;
    instance.otherMapper = otherMapper;
  }
  @Test public void testDoSomething() {
    Entity a = new FakeEntity();
    Entity b = new FakeEntity();
    Comp compA = compMapper.put(a, Comp::new);
    Comp compB = compMapper.put(b, Comp::new);
    Other otherA = otherMapper.put(a, Other::new);
    Other otherB = otherMapper.put(b, Other::new);
    // Setup compA/compB/otherA/otherB for whatever test I need.
  }
}

Yet, it's not possible because:

• I have to mock ComponentMapper to return what I want when I want.
• I have to define some unuseful World variable in every test because Entity is final and its constructor is de facto package private (why is it even protected in the first place?).

So please consider the following to make Artemis unit-test friendly:

• Make Entity an interface or an extensible class.
• Create small helper classes with dummy, basic implementation.
• Document how to do proper unit tests with Artemis.

[piotr-j]

I wouldn't be surprised if Entity went away sooner then later, raw ints are favoured. I don't think testability in this format is something that's a very high on priority list. Usually, systems work on a single set of entities, so you could do something like that instead: public void doSomething(CompA a, CompB b); so you don't have to deal with mappers or entities.

[junkdog]

• I have to mock ComponentMapper to return what I want when I want.

Why do you need to mock ComponentMapper and the entities? Have you checked the existing tests? They won't win any beauty contests - some less than others - but they typically spin up whatever is needed and test against it. Sometimes it's necessary, or it shortens the code, to rely on reflection; I'm not a proponent of opening up public API for the sake of testability, as it makes the API harder to evolve.

I have to define some unuseful World variable in every test

The world is basically responsible for the lifecycle of everything artemis. Some classes work without an active world, but most aspects have too many ties with other artemis state.

If you really want to mock artemis classes, consider using a mocking framework which can rewrite them at runtime: PowerMock is pretty popular, there are probably others too.

Entity - (why is it even protected in the first place?).

It boils down to performance and exposing as few internals as feasibly possible/keeping the public API stable.

[ogregoire]

You don't get it. I meant the constructor is protected. It shouldn't. Package private is how it should be. Not the class itself.

Also, I don't care about the whole lifecycle thing. I want to test my stuff in itself, not the lifecycle. So I shouldn't have to deal with the complete lifecycle in proper unit tests. That's the whole point of unit tests.

Regarding mocks, the existing tests are not what I personnally expect from a unit test and mocking is indeed the best solution. I don't mock myself, of course : I use frameworks.

If an entity could, for instance, simply implement an empty Mappable interface, and ComponentMapper could accept Mappable instead of Entity, it'd be a damn good step already.

Finally, regarding the comment about the signature change, it's quickly impractical when you have 3-4 components for two entities. If I have to write methods with 6-8 (or more) parameters, it's a code smell.

It's not because I write games that I have to forget everything about good practices.

In short, Artemis in its current form is very unit-testing unfriendly and it's a sad thing.

Le mer. 9 déc. 2015 01:12, Adrian Papari notifications@github.com a écrit :

• I have to mock ComponentMapper to return what I want when I want.

Why do you need to mock ComponentMapper and the entities? Have you checked the existing tests? They won't win any beauty contests - some less than others - but they typically spin up whatever is needed and test against it. Sometimes it's necessary, or it shortens the code, to rely on reflection; I'm not a proponent of opening up public API for the sake of testability, as it makes the API harder to evolve.

I have to define some unuseful World variable in every test

The world is basically responsible for the lifecycle of everything artemis. Some classes work without an active world, but most aspects have too many ties with other artemis state.

If you really want to mock artemis classes, consider using a mocking framework which can rewrite classes at runtime: PowerMock https://github.com/jayway/powermock is pretty popular, there are probably others too.

Entity - (why is it even protected in the first place?).

It boils down to performance and exposing as few internals as feasibly possible/keeping the public API stable.

— Reply to this email directly or view it on GitHub https://github.com/junkdog/artemis-odb/issues/411#issuecomment-163063992 .

[junkdog]

Also, I don't care about the whole lifecycle thing.

Ehh, but then how do you expect to test stuff with explicit dependencies on mundane state? It'd be easier to guide you if I knew what you're trying to test.

I don't mock myself, of course : I use frameworks.

Yes, so then you have PowerMock etc.

If you only want to unit test your code, then you'll have to write your code such that it can function independently of artemis.

In short, Artemis in its current form is very unit-testing unfriendly and it's a sad thing.

It can't be that hard... Like I said, we're not polluting public API for the sake of test frameworks, and testing is still very much possible. I'm still at a loss why you're not testing the actual thing, though writing your code so that it works both with and without artemis would solve your problem, would it not?

$ mvn clean install -Pgwttest  | grep -E "Tests\ run.*0$"                                                  
Tests run: 156, Failures: 0, Errors: 0, Skipped: 0
Tests run: 14, Failures: 0, Errors: 0, Skipped: 0
Tests run: 14, Failures: 0, Errors: 0, Skipped: 0
Tests run: 19, Failures: 0, Errors: 0, Skipped: 0
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0
Tests run: 82, Failures: 0, Errors: 0, Skipped: 0
[INFO] Tests run: 31, Failures: 0, Errors: 0, Skipped: 0

[Namek]

:fire: :corn:

[ogregoire]

Ehh, but then how do you expect to test stuff with explicit dependencies on mundane state? It'd be easier to guide you if I knew what you're trying to test.

You're speaking about integration tests, not unit tests. I do integration tests also and then in those tests I use World and the dependency injection and everything. In unit tests, I test only how the class behaves, regardless of its dependencies.

It can't be that hard... Like I said, we're not polluting public API for the sake of test frameworks, and testing is still very much possible. I'm still at a loss why you're not testing the actual thing, though writing your code so that it works both with and without artemis would solve your problem, would it not?

I'm not asking you to "pollute" the public API for the sake of testing, but there are no reason, for instance to keep Entity final. Why is it kept final? So that users are patronized into not mixing entities from different worlds? Your documentation already states that they shouldn't be mixed. So why the patronizing? So IMO, the pollution isn't to be introduced but already introduced in that regards.

Also if you plan on using int instead of Entity, I can still create my own ints without World and that's a good thing, yet I can't create Entity without World.

[DaanVanYperen]

Artemis' modular setup allows very tight scoping of integration tests and with a little scaffolding suffices for most users.

Cost/benefit unit testing friendly API is out of scope at this time, moving this to freezer until the board clears up a bit.