I have seen code committed to OFN with amazing test coverage and also code without tests at all (I am sure I have been on the two sides of this spectrum for sure).
Spree decorators and code complexity were valid and understandable reasons for not writing automated tests. Familiarity with js unit tests was another reason I think.
I think we are getting to a point where we should start saying it’s unacceptable to commit code that is not covered by unit tests of some sort, both for rails and js.
Yes. There may be cases in which the test is extremely difficult and a critical fix needs to be deployed very quickly. In that case we should create a tech-debt issue and prioritise the spec after deployment. If we need to fix something quickly, it means that the functionality is important and should definitely be covered by specs.
Your suggestion means another barrier for community contributors but it’s probably worth it. We will probably have the capacity to help out and maybe even write the spec for them.
I think that this will make our specs grow a lot, especially features specs, and that will make them run much longer. I hope that we will find ways of speeding them up.
Re emergency fixes, yes, leaving specs for later is not ideal although acceptable, I like the idea of creating issues for specs in those cases.
We just had a good example without specs. We need to create an issue and write the specs: https://github.com/openfoodfoundation/openfoodnetwork/pull/5283
Re community contributors, I think we can lower the bar there, I am more concerned about the practices for core devs.
Re long build, writing tests is not always about adding tests but having a good set of tests that do enough testing. It’s not an easy thing to do.
We do have some areas where a lot of tests are being executed but mostly I think we need more unit level tests that are much faster than the features tests. @sauloperez has mentioned this a few times: inverting the test pyramid.
Totally agree @luisramos0 and I think the whole core team sees it that way. We should probably remind ourselves that it is the reviewer’s duty to point out when there are not enough tests. I personally tend not to focus my attention on the tests because most of the times they allright but I shouldn’t.
I don’t know what’s your point of view re code coverage. It’s something we’ve discussed a few times but seems to me that it’ll require some time, which we might not have now. I’m sure there is an issue somewhere related to it. In any case, we all know code coverage is by no means a silver bullet.