Question about test coverage expectations for small behavioral changess

Contributor & Development Discussion scikit-image
1

Hi everyone
I am currently working on PR #8062 which introduces stricter validation for channel_axis in channel_as_last_axis

While updating the code I noticed that even small behavioral changes like raising a new ValueError require adding dedicated tests to maintain close to 100% statement coverage

My question is For relatively small internal validation improvements is it always expected to add explicit tests for every new error path? Or is there some flexibility when the change is defensive and not user facing API behavior?

I just want to better understand the projects expectations around coverage and testing philosophy for small refactors or validation improvements

Thanks

2

I don’t think we have a general policy for this. So usually it comes down to the judgement of the people involved in writing and reviewing changes.

For more complex code or a widely used decorator like this, it’s probably wise to add a basic test. Other than that, it’s very nice to have but not a requirement for a PR to be accepted.

Or is there some flexibility when the change is defensive and not user facing API behavior?

Error raising and warning are to some extend user facing API, but I agree that it’s less visible.

1 Like
3

Thanks for the clarification

That makes sense. Since channel_as_last_axis is widely used, I’ll make sure the new error paths are covered properly

Appreciate the guidance :slightly_smiling_face: