Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Clean up tests #34

Closed
1 of 3 tasks
wschella opened this issue May 20, 2019 · 9 comments · Fixed by #105
Closed
1 of 3 tasks

Clean up tests #34

wschella opened this issue May 20, 2019 · 9 comments · Fixed by #105
Labels
difficulty:challenging Implementing or fixing this will present a doable challenge priority:low test
Milestone

Comments

@wschella
Copy link
Member

wschella commented May 20, 2019

  • Testing between operators is inconsistent
  • Util folder needs documentation and cleanup
  • Util.test needs to be renamed
@wschella wschella added difficulty:challenging Implementing or fixing this will present a doable challenge priority:low test labels May 20, 2019
@wschella wschella added this to the Fully tested milestone May 20, 2019
@jitsedesmet
Copy link
Member

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
Copy link
Member Author

wschella commented Aug 4, 2021

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

wschella commented Aug 6, 2021

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
Copy link
Member

rubensworks commented Aug 6, 2021

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
Copy link
Member Author

wschella commented Aug 6, 2021

All fine by me.

@jitsedesmet
Copy link
Member

jitsedesmet commented Aug 12, 2021

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
Copy link
Member Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
difficulty:challenging Implementing or fixing this will present a doable challenge priority:low test
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants