Fix reflective access in test classes

[RoiEXLab]

This seems to fix the remaining reflective access issues (that are our fault) as reported in #2796

[codecov-io]

Codecov Report

Merging #3322 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #3322      +/-   ##
============================================
+ Coverage     21.96%   21.96%   +<.01%     
- Complexity     5883     5884       +1     
============================================
  Files           834      834              
  Lines         72443    72443              
  Branches      11659    11659              
============================================
+ Hits          15913    15914       +1     
+ Misses        54434    54433       -1     
  Partials       2096     2096

Impacted Files	Coverage Δ	Complexity Δ
.../src/main/java/games/strategy/net/nio/Decoder.java	73.91% <0%> (+1.08%)	25% <0%> (+1%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f92477e...5b40d0b. Read the comment docs.

[ssoloff]

that are our fault

@RoiEXLab Did you happen to find a reference that suggests Mockito no longer supports spying on JDK types (or simply types in a different module) in Java 9? This issue seems related, and it's been marked as a bug. I added an MCVE to see if the Mockito maintainers acknowledge whether what we're doing is supported or not.

[RoiEXLab]

@ssoloff I found this issue as well, but it has a slightly different error message.
If you look at The AccessibilityChanger class you will notice, that it's just a helper class that will make all available fields of an object public (or at least that's my theory).
In any case in order to spy on an object mockito needs to be able to access its fields and therefore it would make sense mockito iterates over all fields making them accessible, resulting in the warning for specific fields that have gotten a more restricted access in Java 9.
The issue got labeled as a bug, but I suppose this is just because nobody bothered to dig deeper into this there. Maybe they will check if spied objects have restricted reflective access in the future when Java 11 is released (the next LTS release), but I don't think this will be a priority, and my "workaround" worksfor now 🤷‍♂️

[ssoloff]

I found this issue as well, but it has a slightly different error message.

@RoiEXLab Are you talking about the field it refers to at the end of the message? That is,

to field java.util.logging.LogManager.props

vs.

 to field java.util.Properties.defaults

I believe that difference is simply because two different types are being spied upon, and thus Mockito encounters a different field on which it attempts to reflectively access.

The issue got labeled as a bug, but I suppose this is just because nobody bothered to dig deeper into this there.

I suspect this issue will affect any attempt to spy on a type in a different module, which means, when TripleA is modularized, you may run into the same issue again (there are currently 17 spies of TripleA types).

Regardless, two of the five changes from spies to mocks in this PR are perfectly fine and a good clean up because those instances (currently) do not rely on any behavior of the concrete implementation.

[RoiEXLab]

I believe that difference is simply because two different types are being spied upon, and thus Mockito encounters a different field on which it attempts to reflectively access.

Yes, that's what I'm thinking as well.

I suspect this issue will affect any attempt to spy on a type in a different module, which means, when TripleA is modularized, you may run into the same issue again (there are currently 17 spies of TripleA types).

Possible, but if we actually do this would indicate a serious design issue. Tests are technically in the same package (the same module as well if we'd introduce them) as their test subjects.
So we'll be probably fine.