Having the hooks installed automatically will help devs catch small inconsistencies, that's a first step for the easy-to-catch things.
The run_tests.pl script in misc4dev will hopefully help to run the tests by anybody. Running the tests on a DB without the sample data or with existing data is already a goal, it's something we are trying to catch during QA. TestBuilder helped us a lot for that. Create the data the tests need and the cleanup/rollback should be the way to write tests. Le lun. 5 déc. 2022 à 18:24, Julian Maurice <julian.maur...@biblibre.com> a écrit : > Yes it is expensive. But manual testing is much more expensive. I'd > rather pay for some CPU time than do manually what a bot can do better > and faster than me. Free services exist too (CircleCI, github actions, ...) > > Pushing to a temporary master branch is not a bad idea (a never-broken > master branch sounds nice), but I think it would happen too late. > Patches would have already been rebased multiple times, tested and > reviewed before we notice a test failure. > > koha-testing-docker feels more like a symptom of the difficulty to run > tests than a satisfying solution. It is probably necessary in order to > run complicated end-to-end tests, but it should not be mandatory to run > simple unit tests. > And ktd is not that easy to use. It can break, and it's not easy to > debug for someone not familiar with docker. > > Don't you think it would be a lot easier if we could run `prove t` (or > `npm test` or whatever) on any dev setup and have the same > failures/successes as Jenkins (minus the complicated tests like those > that require selenium) ? > Of course making that happen is not an easy task, but it should be a > long term goal IMO. > > Also I think no one wants to be the person that refuse a patch because a > comment is misaligned, so if that kind of thing is not automated, "not > good enough code" will continue to be pushed. > > > Le 05/12/2022 à 14:55, Jonathan Druart a écrit : > > I don't think we should run the whole test suite everytime we attach > > patches, that would be very expensive in terms of resources. > > However it would be interesting to have a temporary 'master' branch > > that would become 'master' only if jenkins is happy. > > "master" would never be broken :D > > > > Using koha-testing-docker it's very easy to run tests locally. > > It will be even more easier with > > https://gitlab.com/koha-community/koha-misc4dev/-/issues/58 > > > > Le ven. 2 déc. 2022 à 18:16, Julian Maurice > > <julian.maur...@biblibre.com> a écrit : > >> > >> Hi, > >> > >> It seems to me that several issues could be solved by having the CI run > >> sooner, so authors can have feedback as soon as possible. Something like > >> github's CI integration with pull requests would be amazing to detect > >> common mistakes early and stop wasting human time. We should know if > >> tests pass before pushing to master, not after. > >> We could check tidyness, commit message conventions, code coverage by > >> tests, ... all before another person have to look at it. > >> > >> Also tests are not easy to run locally. They might pass on Jenkins, but > >> they do not on my local setup, so they are basically useless to tell me > >> if I broke something. It also makes writing new tests more difficult > >> than it should. > >> > >> If I wanted to improve Koha developer experience, I would start with > that. > >> > >> My 2 cents. > >> > >> Le 02/12/2022 à 15:42, Jonathan Druart a écrit : > >>> Hi devs, > >>> > >>> I was wondering... How good is your "good enough"? > >>> It's a question I often ask myself, when writing patches or QAing > >>> yours: is it good enough to be into Koha? It does not have to be > >>> perfect or it may never reach master, but it must meet certain > >>> standards. > >>> > >>> When I was RM I tried to put that limit quite high. Not necessarily by > >>> asking the author for improving the follow-up patches, but also by > >>> adding the missing bits myself. > >>> > >>> There are various types of "good enough", depending on what we look > >>> at: good enough to be reviewed, good enough to be tested, to be put in > >>> production, get feedback, try an idea, etc. > >>> > >>> Most of the time, what is driving the limit is answering the > >>> questions "Is it maintainable?" / "Is it future proof?". Making sure > >>> the code you are writing won't be broken inadvertently is very > >>> important and must be part of the reflection. > >>> > >>> Katrin asked the QA team members what were our plans for 23.05. In my > >>> opinion we should enforce this "be future proof" concept. Writing code > >>> is easy, especially in Koha (yes it is!). Writing maintainable and > >>> robust code, following our guidelines is a bit harder. That's why we > >>> have a QA process that tries to catch inconsistencies or edge cases > >>> you may have missed. > >>> But I think we can be even more rigorous, and aim for better > >>> standards. We can elevate our ambitions and do better. When we see the > >>> number of new enhancements we are releasing every 6 months, it shows > >>> well that writing code is definitely not a problem. However sometimes > >>> developers are tempted to abandon their work once they are pushed to > >>> master. Pushed does not mean "done". Providing bug fixes, following-up > >>> with patches when needed, fixing CI jenkins, etc. is part of job > >>> (/fun) > >>> > >>> As a Koha developer for a long time now, I know how frustrating it can > >>> be to be asked for follow-ups/rewrite/tests to have our patches > >>> stamped with the precious PQA mark. But from the other point of view > >>> (RM, RMaints, QA team), I also know it's very frustrating when you are > >>> in charge of the release and you do not get the appropriate follow-up > >>> work once it's pushed to master. > >>> > >>> There are some easy steps to write/review better patches. All have > >>> been discussed already several times, but that can be enforced even > >>> more: > >>> 1. Perltidy (!) This is really a very trivial step. Please perltidy > >>> your code. There are hundreds of commits that have been pushed in the > >>> last months that are not tidy (alignment, indentation, lines too long, > >>> etc.) This can easily be configured in your IDE! [1] > >>> > >>> 2. Provide clean code. As said it's not necessarily easy, but the QA > >>> team and RM are supposed to know if the code is clean regarding Koha > >>> guidelines. If the code is not clean, don't PQA, don't push. Either > >>> clean yourself, or ask the original author of the patch to do it > >>> (explaining to them how it can be improved ofc). > >>> > >>> 3. Squash! I have been away for a couple of months and had to read the > >>> git history to know what I missed. And it was really hard to follow > >>> what was going on. First of all, we are not consistent: the commit > >>> message must tell what the patch is doing, not what the bug was (if > >>> you are writing a bug fix). Then, there are way too many follow-ups: > >>> tidiness, indentation fix, typo, spelling, etc. All those tiny > >>> follow-ups could be squashed into the original patch. We don't need > >>> unnecessary tons of entries in our git log for that. For instance, I > >>> usually add a "JD Amended patch: perltidy" for instance when I tidy > >>> the original patch, to keep track of the modification. Squash can be > >>> done by the original author, the QAer, the RM. So yes, you are losing > >>> one commit in the stats but the git log is easy to read! > >>> We could have an "Amended-by" marker if we really want to add credit > >>> on the dashboard (and/or release notes). > >>> > >>> 4. Run tests. Don't wait for Jenkins to fail. This is valid for the > >>> author and QA. Anticipate the failures by running more tests. If you > >>> are modifying C4::Circulation, then run prove on > >>> t/db_dependent/Circulation*, not only Circulation.t. It will help you > >>> catch edge cases. > >>> When something is pushed, track down jenkins failures that could be > >>> caused by your patches. > >>> > >>> 6. Be strict if you are QAing. Each QA member has their own "good > >>> enough", and the RM as well (either relying on the QAer or providing a > >>> full review). But QA must fail if the code is old Koha style code, or > >>> not "good enough". > >>> > >>> 7. Provide support for failing tests, fix things you broke. The QA > >>> team will be more comfortable with your patches if you show them you > >>> are providing support for your stuff. > >>> It's not because it's pushed that you don't have any more efforts to > >>> make. Provide follow-up patches you promised, provide bug fixes, etc. > >>> We don't have a good way to keep track of such demands, which does not > >>> make tracking easier for devs, QA and RM. Any suggestions? > >>> > >>> 8. QA team MUST NEVER* pass QA a change that is not covered by tests, > >>> never. You should not provide change to modules without tests! > >>> * almost never... > >>> > >>> 9. Stick to existing patterns. We should not have different ways to do > >>> the same thing. We should not have different places where a code is > >>> doing the same thing. Ask for help or advice on the list or IRC before > >>> you start coding. We will be happy to guide you. Even if you are a > >>> regular Koha developer it's not always easy to be aware of the latest > >>> master changes. > >>> We will tell you what's the current good practice, or point you to > >>> examples you could reuse for what you want to implement. > >>> > >>> 10. CI should drive the pushes. No more push if CI is not green. The > >>> more we wait the harder it is to track down the origin of the problem. > >>> Last cycle some jobs have been red for months, and we released > >>> 22.11.00 with D10, D11, D12 marked unstable... > >>> > >>> What will I do next cycle? > >>> All of that, and more. I will track down jenkins failures and > >>> responsibilize developers telling them when they break tests (and > >>> won't fix them anymore as I have been doing for years). > >>> I will raise on the bug reports what could have been improved. Yes, > >>> read that I will be even more annoying (to put it politely) than > >>> before. > >>> > >>> I've noticed that the pre-commit git hook on the wiki has been broken > >>> for more than 3 years. And also caught some core developers that do > >>> not have it in place. I am relying on it to keep Vue files tidy so > >>> it's important to have it set up properly. I am planning to force its > >>> usage for ktd users [2]. Adding more checks to it will help us to > >>> catch inconsistencies from the beginning. > >>> > >>> To summarize, writing code is cheap, maintaining code is way more > >>> expensive! It is easier to get the attention of developers before the > >>> patches are pushed to master than after, so we could be more ambitious > >>> and ask more. > >>> > >>> For discussion :) > >>> > >>> Cheers, > >>> Jonathan > >>> > >>> [1] If you are using vim, open ~/vimrc, add > >>> vmap <F8> :!perltidy -q<CR> > >>> Reload vim, select code in visual mode > >>> [2] https://gitlab.com/koha-community/koha-misc4dev/-/issues/59 > >>> _______________________________________________ > >>> Koha-devel mailing list > >>> Koha-devel@lists.koha-community.org > >>> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel > >>> website : https://www.koha-community.org/ > >>> git : https://git.koha-community.org/ > >>> bugs : https://bugs.koha-community.org/ > >> > >> -- > >> Julian Maurice > >> BibLibre > >> _______________________________________________ > >> Koha-devel mailing list > >> Koha-devel@lists.koha-community.org > >> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel > >> website : https://www.koha-community.org/ > >> git : https://git.koha-community.org/ > >> bugs : https://bugs.koha-community.org/ > > _______________________________________________ > > Koha-devel mailing list > > Koha-devel@lists.koha-community.org > > https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel > > website : https://www.koha-community.org/ > > git : https://git.koha-community.org/ > > bugs : https://bugs.koha-community.org/ > > -- > Julian Maurice > BibLibre > _______________________________________________ > Koha-devel mailing list > Koha-devel@lists.koha-community.org > https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel > website : https://www.koha-community.org/ > git : https://git.koha-community.org/ > bugs : https://bugs.koha-community.org/ >
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : https://www.koha-community.org/ git : https://git.koha-community.org/ bugs : https://bugs.koha-community.org/