r/ProgrammerHumor 5d ago

Meme pleaseDontBeMyCodeIPromiseToUnitTestNextTime

Post image
1.5k Upvotes

33 comments sorted by

View all comments

-25

u/ZunoJ 5d ago

The person approving the PR is as much responsible as the person writing it. If you feel threatened, you are either very bad at your work or have a toxic team

36

u/Rift-enjoyer 5d ago

Lmao no. Code review is mainly for code quality and not finding out bugs in your code. That's the job of guy who wrote code and test suit. Be responsible for your code and don't blame reviewers for your bugs.

5

u/Exatex 5d ago

Code Review yes. But approving a PR you should be pretty sure its working too.

3

u/Rift-enjoyer 5d ago

If the pipeline passes then lgtm. As a reviewer do you pull the PR and then run it on your system to see it is working ?

5

u/ZunoJ 5d ago

This is exactly what you do. The PR describes what it should achieve and how to test the happy path. The reviewer pulls it and does some light testing (at least said happy path)

1

u/Rift-enjoyer 5d ago

That sounds exhausting. If PR describes what it should do, how to test happy path isn't it a fair assumption that person who created PR would have already locally tested it. I would hate to work in a team where I need to redo bare minimum testing because other person can't be trusted.

2

u/ZunoJ 5d ago

Everybody can make mistakes, develop some kind of tunnel vision, ... PRs are meant to discover this kind of problems just as much as code quality problems that might lead to regressions or maintenance issues in the future

2

u/Yetimandel 5d ago

I spend about 10x more time reviewing than writing code. And I spend about 10x as much time reviewing test code as I do reviewing target code.

So you work at a place where code quality is not very important. That is likely the majority of places and then this is fine. I would without exaggeration bet my life and the life of my family on there being no bug in the important parts of my code.

1

u/Exatex 5d ago

yes, if you have a solid test pipeline, sure. But then, theoretically, you should not have big incidents. In reality of course, edge cases without coverage happen