Skip to content

Add eval coverage#436

Open
cb341 wants to merge 1 commit intomainfrom
feature/eval-coverage
Open

Add eval coverage#436
cb341 wants to merge 1 commit intomainfrom
feature/eval-coverage

Conversation

@cb341
Copy link
Contributor

@cb341 cb341 commented Feb 13, 2026

Since a couple years time, simplecov supports eval coverage.

This allows us to also check for branch and line coverage within erb files.
Most importantly views html.erb. I propose we check the eval coverage in new applications by default.

Related PRs:

@cb341 cb341 self-assigned this Feb 13, 2026
@sislr sislr requested a review from coorasse February 13, 2026 13:54
Copy link
Member

@coorasse coorasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the last discussion, we should enable grouped coverage.

@cb341
Copy link
Contributor Author

cb341 commented Feb 19, 2026

@coorasse
From the last discussion, we should enable grouped coverage.

Why? I don't see how this relates to the app setup for eval coverage.
I'd enforce 100% line / code coverage regardless of source.

@cb341
Copy link
Contributor Author

cb341 commented Feb 19, 2026

The coverage groups would mainly be relevant in existing projects when we introduce eval.

@coorasse
Copy link
Member

I am a bit afraid of being too strict. But you are right! Let's see if that's fine also for others

@cb341
Copy link
Contributor Author

cb341 commented Feb 25, 2026

@coorasse
I am a bit afraid of being too strict. But you are right! Let's see if that's fine also for others

Can't we say the same about the 100% line and branch coverage requirements on ruby files too?

@coorasse
Copy link
Member

coorasse commented Mar 1, 2026

From my point of. view most of the business logic, which is what I mostly care about stays in rb files. I'd expect the little logic that stays in the views to be more frontend-related, and therefore not that business critical.
One example: checking if the page displays "no results" when there's no results, is not as business critical as everything else, and I'd not like to force the developer to test it but rather leave it to personal judgement.
Once said said, I am one of many seniors engineers here, if somebody has a strong preference in an opposite direction, I am very much open.

@renuo renuo deleted a comment from lukasbischof Mar 2, 2026
@schmijos schmijos removed their request for review March 2, 2026 09:56
Copy link
Member

@rnestler rnestler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since one usually doesn't have much logic in the templates the coverage should be easy to reach. And if not everything is covered it is helpful to know since it can indicate:

  • Dead / unreachable code in the template
  • Edge cases that were forgotten to test
  • To much logic in the templates such that it gets hard to test all edge cases

Only downside is probably a bit longer test run?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants