4 ms·

> Writing databases & low-level replication tools involves nuance and simple one line changes can have profound and unexpected changes in correctness and perfor

> Writing databases & low-level replication tools involves nuance and simple one line changes can have profound and unexpected changes in correctness and performance. Small contributions typically required hours of my time to properly test and validate them ... I've made the decision to keep this project closed to contributions for my own mental health and long term viability of the project. While I understand the burden of accepting and evaluating code from others and the very real possibility of a burn-out due to it, can't test-driven development (or even Behavior-driven development) reduce a lot of this burden? (Related: How SQLite is tested - https://www.sqlite.org/testing.html https://www.sqlite.org/testing.html ).

Aeolun 6y ago
Yeah, just ignore any PR that doesn’t pass the tests. Also a good way to weed out the people that don’t care enough to fix it if they break something.
- swiftcoder 6y ago
  You also have to reject any PR that modifies the tests. Otherwise the maintainer now has to validate test suite integrity any time they accept a PR.
  - hoseja 6y ago
    I don't understand the obsession with testing. You can't test for unknown unknowns, that is, non-obvious bugs. Sure it's nice "automated documentation" but I feel like the reverence it gets is overstated.
    - webmobdev 6y ago
      From what I understand - you don't test for all unknowns as that is humanly impossible. You test for realistic "known" only. This largely prevents regression bugs as new fixes or features are added. When a bug reveals itself and is fixed, it becomes a known case that you can decide to create a test for (if necessary) and prevent its recurrence in the future. Personally, I feel it improves the code quality and readability for others as the tests also help them better understand the functionality of the code within a project.
  - webmobdev 6y ago
    Does that really happen that often? Best practice is you only add more tests and don't touch existing tests unless absolutely necessary.
    - swiftcoder 6y ago
      Any change that involves a refactor is going to end up modifying the tests. The higher your test coverage, the more the test suite churns.
- jabbany 6y ago
  This is a good _start_ but nowhere near being sufficient for reducing workload of reviewing PRs. If a patch is a bugfix then clearly none of the past tests caught the bug <_<... If the patch is a new feature then it will certainly need new tests to validate its own behavior is correct...
  - cbm-vic-20 6y ago
    If a patch is a bugfix, it should include a test that fails before the patch, but passes after the patch (along with all of the other tests).
    - jabbany 6y ago
      Yes, the idea is that the effort isn't really eliminated. It's just shifted from reviewing the code to reviewing the new tests, which sometimes can be just as hard as reviewing the code when complex bugs are involved.
- josephg 6y ago
  I’ve used this in the past and its great. Most PRs change something that requires new tests to be written, so I have a rule that any PRs must add unit tests verifying the new behaviour. If your feature really matters to you then it also matters that it’s correct. I don’t want to spend my time triaging bugs in a feature I didn’t write and don’t use. For some reason this rule weeds out about 80% of contributions - including almost all low quality PRs. It’s delightful.
  - jabbany 6y ago
    I have an anecdote contributing to a testing framework (of all things) that had similar policies. Turns out it's pretty hard to write tests making sure something asynchronously executed never happens sans introducing (flaky) "timeout"-based tests. At some point the maintainers were like, "yeah, this draft PR looks like it's doing the right thing so let's just merge it without a working behavior test"
    - josephg 6y ago
      Yeah I make exceptions too when the PR is something I personally care about. (Or I sometimes help out writing tests when someone finds a high priority bug they struggle to reproduce). But I think its a good baseline expectation. Keeping code bug-free is difficult work.
- auggierose 6y ago
  What if the bug fix is right and the tests are wrong?
  - webmobdev 6y ago
    It's easier to spot bugs in tests.
icegreentea2 6y ago
It can be hard to get TDD/BDD to cover enough of the aspects of a code-base (depending on what the code is doing). SQLite is special particularly in just how comprehensive their test suite. In particular, note that SQLite's full test suites is very much their secret sauce (and is not open-source). You can get a sense of the amount of effort it takes the create and maintain that type of test suite for something like SQLite.

Aeolun 6y ago

Yeah, just ignore any PR that doesn’t pass the tests. Also a good way to weed out the people that don’t care enough to fix it if they break something.

swiftcoder 6y ago
You also have to reject any PR that modifies the tests. Otherwise the maintainer now has to validate test suite integrity any time they accept a PR.
- hoseja 6y ago
  I don't understand the obsession with testing. You can't test for unknown unknowns, that is, non-obvious bugs. Sure it's nice "automated documentation" but I feel like the reverence it gets is overstated.
  - webmobdev 6y ago
    From what I understand - you don't test for all unknowns as that is humanly impossible. You test for realistic "known" only. This largely prevents regression bugs as new fixes or features are added. When a bug reveals itself and is fixed, it becomes a known case that you can decide to create a test for (if necessary) and prevent its recurrence in the future. Personally, I feel it improves the code quality and readability for others as the tests also help them better understand the functionality of the code within a project.
- webmobdev 6y ago
  Does that really happen that often? Best practice is you only add more tests and don't touch existing tests unless absolutely necessary.
  - swiftcoder 6y ago
    Any change that involves a refactor is going to end up modifying the tests. The higher your test coverage, the more the test suite churns.
jabbany 6y ago
This is a good _start_ but nowhere near being sufficient for reducing workload of reviewing PRs. If a patch is a bugfix then clearly none of the past tests caught the bug <_<... If the patch is a new feature then it will certainly need new tests to validate its own behavior is correct...
- cbm-vic-20 6y ago
  If a patch is a bugfix, it should include a test that fails before the patch, but passes after the patch (along with all of the other tests).
  - jabbany 6y ago
    Yes, the idea is that the effort isn't really eliminated. It's just shifted from reviewing the code to reviewing the new tests, which sometimes can be just as hard as reviewing the code when complex bugs are involved.
josephg 6y ago
I’ve used this in the past and its great. Most PRs change something that requires new tests to be written, so I have a rule that any PRs must add unit tests verifying the new behaviour. If your feature really matters to you then it also matters that it’s correct. I don’t want to spend my time triaging bugs in a feature I didn’t write and don’t use. For some reason this rule weeds out about 80% of contributions - including almost all low quality PRs. It’s delightful.
- jabbany 6y ago
  I have an anecdote contributing to a testing framework (of all things) that had similar policies. Turns out it's pretty hard to write tests making sure something asynchronously executed never happens sans introducing (flaky) "timeout"-based tests. At some point the maintainers were like, "yeah, this draft PR looks like it's doing the right thing so let's just merge it without a working behavior test"
  - josephg 6y ago
    Yeah I make exceptions too when the PR is something I personally care about. (Or I sometimes help out writing tests when someone finds a high priority bug they struggle to reproduce). But I think its a good baseline expectation. Keeping code bug-free is difficult work.
auggierose 6y ago
What if the bug fix is right and the tests are wrong?
- webmobdev 6y ago
  It's easier to spot bugs in tests.

icegreentea2 6y ago

It can be hard to get TDD/BDD to cover enough of the aspects of a code-base (depending on what the code is doing). SQLite is special particularly in just how comprehensive their test suite. In particular, note that SQLite's full test suites is very much their secret sauce (and is not open-source). You can get a sense of the amount of effort it takes the create and maintain that type of test suite for something like SQLite.