beanshell / beanshell

Beanshell scripting language
Apache License 2.0
815 stars 183 forks source link

Fix for issue 725 casts on numeric references types causing exceptions #729

Open opeongo opened 1 year ago

codecov[bot] commented 1 year ago

Codecov Report

Attention: 656 lines in your changes are missing coverage. Please review.

Comparison is base (b1998b1) 74.24% compared to head (4e8d9dd) 74.23%. Report is 1024 commits behind head on master.

Files Patch % Lines
src/main/java/bsh/Interpreter.java 85.20% 50 Missing and 21 partials :warning:
src/main/java/bsh/BshClassManager.java 65.73% 48 Missing and 13 partials :warning:
src/main/java/bsh/NameSpace.java 87.87% 40 Missing and 17 partials :warning:
src/main/java/bsh/ClassGeneratorUtil.java 89.59% 35 Missing and 14 partials :warning:
src/main/java/bsh/BshMethod.java 76.73% 26 Missing and 21 partials :warning:
src/main/java/bsh/Name.java 86.76% 20 Missing and 23 partials :warning:
src/main/java/bsh/LHS.java 74.35% 28 Missing and 12 partials :warning:
src/main/java/bsh/Invocable.java 83.03% 23 Missing and 15 partials :warning:
src/main/java/bsh/DelayedEvalBshMethod.java 61.97% 26 Missing and 1 partial :warning:
src/main/java/bsh/BSHPrimarySuffix.java 84.41% 17 Missing and 7 partials :warning:
... and 30 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #729 +/- ## ============================================ - Coverage 74.24% 74.23% -0.02% - Complexity 3041 3044 +3 ============================================ Files 108 108 Lines 9354 9360 +6 Branches 1857 1860 +3 ============================================ + Hits 6945 6948 +3 Misses 2070 2070 - Partials 339 342 +3 ``` | [Flag](https://app.codecov.io/gh/beanshell/beanshell/pull/729/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beanshell) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/beanshell/beanshell/pull/729/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beanshell) | `74.23% <87.44%> (-0.02%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=beanshell#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nickl- commented 1 year ago

Looks good but we are loosing too much code coverage. Please add tests for the other paths too:

Some or all of these conditions are missing:

1.

fromType == null

2.

fromType.isPrimitive()

3.

!toType.isAssignableFrom( fromType )

4.

checkOnly == true

I know it is getting harder to maintain or even improve on the overall coverage, but the unit tests only cover a 3rd of this patch.

opeongo commented 11 months ago

Looks good but we are loosing too much code coverage. Please add tests for the other paths too:

Some or all of these conditions are missing:

fromType == null
fromType.isPrimitive()
!toType.isAssignableFrom( fromType )
checkOnly == true

I know it is getting harder to maintain or even improve on the overall coverage, but the unit tests only cover a 3rd of this patch.

It is not easy to write tests for all of these conditions. For example, how to I write a test for checkOnly when it is not exposed to the scripting environment?