I was trying to reduce the available memory for running unit tests, in the hopes that it will reveal our problems.

Managed to reduce the memory required to run the tests, where all tests will now succeed within 15MB of heap and the bsh scripted tests alone can complete in only 11MB of heap space.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-surefire-plugin</artifactId>
    <version>3.0.0-M5</version>
    <configuration>
        <trimStackTrace>false</trimStackTrace>
        <argLine>-Xmx11M -XX:+UseSerialGC ${surefireArgLine}</argLine>
        <systemPropertyVariables>
            <bsh.debugClasses />
            <accessibility>false</accessibility>
            <script />
            <verbose>false</verbose>
            <skip_known_issues>true</skip_known_issues>
        </systemPropertyVariables>
    </configuration>
</plugin>

mvn test -Dverbose=true -Dtest=BshScriptTestCase

This does nothing for speed of course, java has its hands full keeping things running, any less resources and we start getting out of memory errors. The first of which comes from the super import import *; which stumbles in BshClassPath. We can probably try and squeeze more but it doesn't break per se, there simply isn't anymore room to create new stuff.

I cleaned up BshClasspath, mostly applying generic types to make it easier to read. Made a few tweaks and there is probably more we can do for this but I am confident it is not the cause for any of the issues. Improved the coverage too so it should be on par now.

Circling back to the loader changed listeners I discovered this snippet on BSHType:

https://github.com/beanshell/beanshell/blob/b14e762c8904c088ca4a1e6b3dcadd629cde4b43/src/main/java/bsh/BSHType.java#L175-L179

It appears to be from the first commit and I am not sure whether it is only theoretical or based on actual issues. The comments it refers to I found on SimpleNode:

https://github.com/beanshell/beanshell/blob/b14e762c8904c088ca4a1e6b3dcadd629cde4b43/src/main/java/bsh/SimpleNode.java#L31-L47

It is true that we reiterate over the nodes, and certain interpreted results are cached as instance variables. We cannot cache everything or it will stop appearing to execute. As for scripted types, already interpreted, and executed at least once, I cannot imagine what would entice them to change on the fly, regardless of actions on the classpath. If the types are wrong then I would imagine restarting the script would be the course of action, ensuring that the classpath is appropriate before interpretation and execution.

Anyway, unsubscribing from the publisher does not break any tests, and halves the number of listeners to announce to from the class manager. The "cached" types state are not thread safe and if they were to change on the fly it could only spell disaster. Unfortunately this is also only theoretical and we will need to test it in the wild. Commented out for now to see what explodes.

Commit log message

low memory unit tests

optimised unit test memory usage, fixed deprecation fixed two incomplete tests, added coverage for BshClassPath return exit status in exec command as per #109
fix deprecated File.toURL in addClassPath command
Cleaned up BshClassPath with minor refactoring
Unsubscribe BSHType from class loader changed broadcasts

Codecov Report

Merging #669 (0a47203) into master (b14e762) will decrease coverage by 0.10%. The diff coverage is 89.13%.

:exclamation: Current head 0a47203 differs from pull request most recent head 76cdb97. Consider uploading reports for the commit 76cdb97 to get more accurate results

@@             Coverage Diff              @@
##             master     #669      +/-   ##
============================================
- Coverage     71.18%   71.08%   -0.11%     
+ Complexity     2776     2772       -4     
============================================
  Files           106      106              
  Lines          9072     9071       -1     
  Branches       1778     1780       +2     
============================================
- Hits           6458     6448      -10     
- Misses         2202     2205       +3     
- Partials        412      418       +6

Flag	Coverage Δ
unittests	`71.08% <89.13%> (-0.11%)`	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/main/java/bsh/classpath/BshClassPath.java	`84.29% <88.88%> (-1.20%)`	:arrow_down:
src/main/java/bsh/BSHType.java	`80.59% <100.00%> (-1.10%)`	:arrow_down:
src/main/java/bsh/classpath/ClassManagerImpl.java	`64.70% <100.00%> (-3.53%)`	:arrow_down:
src/main/java/bsh/Name.java	`86.60% <0.00%> (+0.62%)`	:arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

beanshell / beanshell

Running unit tests with restricted heap size #669

Codecov Report