autoconsent icon indicating copy to clipboard operation
autoconsent copied to clipboard

Implement first unit tests (#333)

Open freiondrej opened this issue 7 months ago • 4 comments

@muodov Hello and nice to meet you! 👋 (or any other maintainers if you're there - sorry, I'm not ignoring you on purpose!)

This is a very rough draft, I aimed to push it early as I believe discussing concrete code is always easier than just thinking about code to be written. Therefore, I'm fully ready to redo it 100%, I'm definitely not emotionally attached to it 😁

A couple of notes / questions:

  • I used vitest, mainly because it has out of the box TS support and I have previous experience with it, but no other strong reasons for it. I initially looked if playwright could do the job as it's already in the project, but from what I've found it can be a bit hacky.

  • I used quite a lot of mocking. I'll experiment with using test doubles instead, maybe that will be a better fit. My slight worry with test doubles is that we might lean too much in the direction of integration tests if we go there, and your issue specifically mentioned unit tests.

    • The main problem I see with mocks is that they make the test quite coupled to implementation = I see a high chance of having to redo the tests with any refactoring to the source. That might be ok or not - would like to hear your take. (Btw as discussions of mock vs non-mock approaches to testing can often get quite heated, don't worry, I've been there, I'm fine with both approaches :))
  • As my initial version stands, the maintainability would be quite poor. I'd definitely look into extracting some common patterns, helper functions etc to make it more clear how each test case differs from the others.

    • EDIT: maybe it would make sense to make assertions to the state as it was being set over time, or perhaps the sendContentMessage calls? That might significantly increase readability.
  • I aimed to cover the logic you mentioned in the issue description. Would it make sense to cover all methods within the class?

Thanks! 🙌

freiondrej avatar Jul 26 '24 14:07 freiondrej