Clean up tests - Githubissues

wschella commented 5 years ago

[x] Testing between operators is inconsistent
[ ] Util folder needs documentation and cleanup
[ ] Util.test needs to be renamed

jitsedesmet commented 3 years ago

I was looking at this issue and I wonder whether you could provide some more information on the tasks you mentioned. I was also looking at the tests and noticed there's quite some similarity between testAll in combination with testAllErrors and the classes in the Truthtable file. I wonder if you could clarify the difference between them? If they are historically developed and provide (almost) the same service, I would want to remove them. Less is more after all.

wschella commented 3 years ago

I think it's a bit of both, testAll+testAllErrors is a bit more straightforward (in implementation and use) then the TruthThable, it doesn't require the aliasing maps, and doesn't hide the operator. The cases in testAll(Errors) are simply correct expressions, while the Truthtable is more like a parametrized expression. It think it's sensible to have both, but this difference is not very clear from naming. documentation, and API.

If you can abstract this decently, I'm definitely not opposed to it, e.g. with naming testAll, testAllErrors, and testAllParametrized would already make more sense. If you can then make the API a bit more similar and reuse some logic. I bet there's inspiration to be gained from test framework conventions.

In regards to the points in the issues:

[] Testing between operators is inconsistent. I have no clue anymore.
[] Util folder needs documentation and cleanup. As you can see, the test organization is a bit of a mess. I managed to get a nice structure for writing tests easily, and then I had enough, but behind the hood it needs structure & clarity. I think most files need at least some documentation on how they are used. The organization can be cleaned up. You know, make this something you can confidently pass to the next person.
[] Util.test needs to be renamed. This issue is broader, the name "util.test" exists only to differentiate from the "util" folder, which is utilities for testing. I think following structure might make more sense:

- lib/    # Testing structure matching the lib organization
  - evaluators/
  - functions/
  - ... 
- spec/   # The spec tests
- util/   # Test utilities

where misc can be at the root, or in lib/

jitsedesmet commented 3 years ago

Thank you for the clarification! I will consider Testing between operators is inconsistent as done unless I were to suddenly understand what you meant by it.

rubensworks commented 3 years ago

I would suggest keeping everything dist-related in lib/, and everything test-related in test/ So perhaps something more like this:

- lib/
  - evaluators/
  - functions/
  - util/
  - ... 
- test/
  - unit/   # subdirs matching the lib/ dir
  - spec/   # The spec tests
  - util/   # Test utilities

wschella commented 3 years ago

I explained myself badly, this was what I meant, but then:

- lib/
  - evaluators/
  - functions/
  - util/
  - ... 
- test/
  - lib/   # subdirs matching the lib/ dir
  - spec/   # The spec tests
  - util/   # Test utilities

with lib instead of unit (but I have no preference, although they're not really unit tests, they go end-to-end much like an integration test)

rubensworks commented 3 years ago

Ah, no unit tests indeed (but we'll probably need them later for achieving full coverage)

with lib instead of unit (but I have no preference, although they're not really unit tests, they go end-to-end much like an integration test)

Perhaps we can just call it integration/ then?

So that would then become:

- lib/
  - evaluators/
  - functions/
  - util/
  - ... 
- test/
  - integration/   # Subdirs matching the lib/ dir
  - spec/          # The spec tests
  - util/          # Test utilities

wschella commented 3 years ago

All fine by me.

jitsedesmet commented 3 years ago

Thank you for the clarification! I will consider Testing between operators is inconsistent as done unless I were to suddenly understand what you meant by it.

It might be that this inconsistency is solved by using runTestTable? All tests use the same interface? which is consistent?

wschella commented 3 years ago

It might be that this inconsistency is solved by using runTestTable? All tests use the same interface? which is consistent?

Very likely yes. Anyway, if we don't know what it means by now, we can close it.

comunica / sparqlee

Clean up tests #34