Discussion
Loading...

Discussion

Log in
  • About
  • Code of conduct
  • Privacy
  • Users
  • Instances
  • About Bonfire
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 8 hours ago

@pierstoval At work, we have some specialized cyclomatic complexity code sniffs that factor in symbols like `??=` into the complexity score of a function.

  • Copy link
  • Flag this post
  • Block
Dmitri Goosens :elephpant:
Dmitri Goosens :elephpant:
@dgoosens@phpc.social  ·  activity timestamp 8 hours ago

@pierstoval

how would that work with

```php

$object->someProp()
??= 'default' ;

```
?

  • Copy link
  • Flag this comment
  • Block
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 8 hours ago

@dgoosens @pierstoval I think that still gets treated as one line. In the report, you might see the first line as green and the second as white/clear, though, instead of red/pink.

  • Copy link
  • Flag this comment
  • Block
Alex Rock
Alex Rock
@pierstoval@mastodon.social  ·  activity timestamp 8 hours ago

@ramsey @dgoosens Yep, this kind of statement is always considered to be one line. In my memories of how coverage is handled, it's per-line, all the time, therefore a multi-line statement is still one line, which can make one-liners become really annoying

  • Copy link
  • Flag this comment
  • Block
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 8 hours ago

@pierstoval Does branching coverage catch the conditional?

  • Copy link
  • Flag this comment
  • Block
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 8 hours ago

@pierstoval I guess I could tag @derickr or @sebastian because either one probably knows. 🙂

Also, I think I called it the wrong thing. I think it’s normally referred to as “path coverage.”

  • Copy link
  • Flag this comment
  • Block
Sebastian Bergmann :phpunit:
Sebastian Bergmann :phpunit:
@sebastian@phpc.social  ·  activity timestamp 8 hours ago

@ramsey Both "Branch Coverage" and "Path Coverage" exist, see https://phpunit.expert/articles/path-coverage-or-mutation-testing.html?ref=mastodon for details.

phpunit.expert

Path Coverage or Mutation Testing?

How thoroughly do your tests cover the code, and how reliably do they detect real errors? I show you how to find out.
⁂
More from
Sebastian Bergmann :phpunit:
Sebastian Bergmann :phpunit:
  • Copy link
  • Flag this comment
  • Block
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 4 hours ago

@sebastian Thanks! I didn’t realize they were two separate things.

Can one of them show if you haven’t tested all conditions of `??=` ?

  • Copy link
  • Flag this comment
  • Block
Alex Rock
Alex Rock
@pierstoval@mastodon.social  ·  activity timestamp 8 hours ago

@ramsey I'm also sure that @derickr and @sebastian know about it 😁

Still, I don't think it's even possible to break coverage into partial statements like that, it would need a much harder check at runtime, implying current context from the call stack, etc., which I think coverage tools can't actually cover.

It also implies changing how current code coverage is stored and displayed, and even difftools sometimes struggle with single-line diffs...

But maybe it's possible, I don't know 😅

  • Copy link
  • Flag this comment
  • Block
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 8 hours ago

@pierstoval At work, we have some specialized cyclomatic complexity code sniffs that factor in symbols like `??=` into the complexity score of a function.

  • Copy link
  • Flag this comment
  • Block
Juliette
Juliette
@jrf_nl@phpc.social  ·  activity timestamp 4 hours ago

@ramsey Specialized sniffs ? Why not use the `Generic.Metrics.CyclomaticComplexity` sniff ? It already takes null coalesce into account.

No doubt there may be (other) things which could be improved, but I'll happily review a PR to get that sorted, if so submitted.

/cc @pierstoval

  • Copy link
  • Flag this comment
  • Block
Juliette
Juliette
@jrf_nl@phpc.social  ·  activity timestamp 4 hours ago

@ramsey Just curious to hear why, not meant as criticism.

  • Copy link
  • Flag this comment
  • Block
Ben Ramsey
Ben Ramsey
@ramsey@phpc.social  ·  activity timestamp 3 hours ago

@jrf_nl Potentially two reasons:

1. The sniff we have is very, very old and maybe predates the Generic one?
2. NIH syndrome

This code base is over 20 years old, and along the way it has had thousands of different developers working in it, so who really knows?

  • Copy link
  • Flag this comment
  • Block
Juliette
Juliette
@jrf_nl@phpc.social  ·  activity timestamp 3 hours ago

@ramsey Understood. If they'd be interested, I could take a look and suggest some optimizations.

P.S.: just checked - the Generic sniff was introduced in 2007 ;-)

  • Copy link
  • Flag this comment
  • Block

bonfire.cafe

A space for Bonfire maintainers and contributors to communicate

bonfire.cafe: About · Code of conduct · Privacy · Users · Instances
Bonfire social · 1.0.2-alpha.29 no JS en
Automatic federation enabled
Log in
  • Explore
  • About
  • Members
  • Code of Conduct