When to write tests?

[Akuli]

From #1371:

Porcupine isn't a project where you need to write a test for everything.

😞

Let's discuss when and why tests should be written, and when and why they should not be written.

I think we should write tests when:

• The code is hard to get right when writing it initially.
• It is easy to break the code in unintended ways when you try to modify it.
• The test is simple, easy to understand, and easy to get right when writing it initially.
• There either isn't other higher-level code that invokes the code we're testing, or there is but the higher-level code fails in mysterious ways when the lower-level code is broken.
• If the feature was broken, it would take a long time for Porcupine developers to notice it.

A good example of this is the find plugin. It has lots of corner cases that must behave in exactly the right way for it to not annoy me. Some corner cases I would notice immediately when trying to do real work with Porcupine, but others I would not. All corner cases are easy to test though, since you mostly need to insert stuff to entries, press buttons, and check what got tagged in the text widget.

I think we should NOT write tests when:

• The code is trivially simple.
• There are no surprising corner cases that you would need to be particularly careful about, or you can explain them in a short comment. (Nobody reads long comments.)
• Testing the code would be difficult.
• If the feature was broken, Porcupine developers would notice it when trying to use Porcupine for something and immediately create an issue.

A good example of this is UI details, such as padding, font sizes, window sizes, window titles and the like. To me, pull requests like #794 check all the boxes for something that doesn't need tests.

I also think that the number of lines of code shouldn't be used to determine whether tests are a good idea or not. Sometimes you need 50 lines of tests for a one-line regex, sometimes you need 1000 lines of code to implement autocompletions with a langserver even though a 20 lines long test verifies that completions are working in a couple different situations.

Thoughts?

[benjamin-kirkbride]

I think we should NOT write tests when:

Are you saying "under these conditions tests are unnecessary" or are you saying "under these conditions, tests are bad/unwanted/would not be accepted"?

Testing the code would be difficult.

Does this override the "I think we should write tests when" section? As in, if tt is easy to break the code in unintended ways when you try to modify it, but it's also hard to test, are you saying we shouldn't test it? or that it could possibly be justified in not having a test? Or something else?

If the feature was broken, Porcupine developers would notice it when trying to use Porcupine for something and immediately create an issue.

This is kind of sticky IMO. My intentions (as of now) for Porcupine are not going to have a ton of overlap with others use, so I want to be able to trust that the test harness will cover enough things that I don't have to click around and see if anything seems out of place or broken.

---

My ":disappointed:" was mostly tongue in cheek, but I think that we do have a somewhat conflicting view of tests. I like to write tests first, and then make the code work to match it. This can result in a lot of tests, both discrete tests, and parameterized tests (and property based tests a la #1378). I run the tests a lot when I'm developing, which is why I am really wanting to figure out #1329

I don't think that there is a such thing as "too many" tests, with basically one caveat: you don't want dubious tests which are fragile which make it hard/annoying to change things later. That said, it's easy enough to remove tests, and I try to make sure that my tests are labeled in a way that makes it clear what the intent of the tests are, so future devs (including me) have some idea of if a failing test means something is broken vs means that the code has changed such that the test is irrelevant.

To be clear, I'm not saying we should start enforcing "100% code coverage" or anything like that.

---

I think the action item of this issue is to have some sort of policy/rationale on testing documented in CONTRIBUTING.md

[rdbende]

I like to write tests first, and then make the code work to match it.

I don't think that TDD with tkinter is a good idea.
When you develop a GUI app, you first want to see if the behavior is right, but testing a GUI (especially with tkinter, especially cross-platform -> #1379) is quite tricky, and it's hard to get right initially. You first want to have something functional, and testing (if possible/necessary) can come later. Obviously you want to test for corner cases, that most users would never come across anyway, but for a GUI I find that it's easier this way.

[benjamin-kirkbride]

@rdbende I think that it depends on the context of what is being worked on. Most of what I have done so far is things like, make a regex, refactor existing "commands" into the new Actions paradigm, fix bad assumptions about how tests should run, etc.

I don't disagree that TDD (I was avoiding that term because a lot of people have a bad association with it 😅) is harder and often not worth it with GUI programming, but a lot of stuff in Porcupine, especially when it comes to plugins, is not really GUI programming. That's my opinion at least :)

[ThePhilgrim]

I'm not very experienced working with projects that use tests at all (I've done one project with proper testing). However, to me, it would make sense if as many plugins as possible in Porcupine are tested, as newly written plugins could potentially break existing ones (I would assume?). This can easily be missed if the plugins are not properly tested.

[Akuli]

The tl;dr for me is that I agree with everything everyone is saying. Maybe the correct solution should be "use your own best judgement" instead of some hard-coded rules :)

---

"under these conditions tests are unnecessary" or are you saying "under these conditions, tests are bad/unwanted/would not be accepted"?

More like that they are unnecessary, but ultimately I go on "practicality beats purity" / "case-by-case" basis. I also won't be very strict about deleting unnecessary tests.

if it is easy to break the code in unintended ways when you try to modify it, but it's also hard to test, are you saying we shouldn't test it?

This is a gray area, and can go either way to me. But the situation sounds like the code is so fragile that it needs tests, whether it is easy to write them or not.

My intentions (as of now) for Porcupine are not going to have a ton of overlap with others use

Go ahead and write tests for features like this. Though I'm sure everyone will love the Command Palette and many of us will use it daily :)

TDD (I was avoiding that term because a lot of people have a bad association with it 😅 )

I'm really happy that we can discuss these things with reasoning, and it doesn't turn into a mindless yelling match along the lines of "TDD sucks" ❤️

[Moosems]

I too enjoy the ability for civil discussion however I must now joke that this that and the other suck XD!

[benjamin-kirkbride]

Go ahead and write tests for features like this. Though I'm sure everyone will love the Command Palette and many of us will use it daily :)

I'm not actually talking about the Command Palette or anything I'm currently working on (except maybe the toolbar).

This will eventually be the topic of another issue, but medium/long term I plan on using Porcupine as a sort of front end for a platform for journalling/personal knowledge-base I want to build for myself, hence my focus on Markdown and improving the way that plugins get added and interact with eachother.