StubToolkit must go!

[Jim Graham]

The problem with this approach is that you are testing PolygonMock 
instead of Polygon.  Any bugs in the impl_ methods are untestable when 
you override them, and any future changes to Polygon which modify the 
way that the impl_ methods are used may introduce bugs that do not 
impact the testing you do with your Mock version because it hides those 
changes.

Also, the impl_ methods are being slowly removed in favor of the 
Accessor paradigm.  Many impl_ methods have already disappeared.  These 
public implementation methods have always been a hack and they are 
currently only public (and overridable) because we haven't switched over 
to a better (and easier to keep secure in the long run) paradigm.  Your 
testing methodology would require us to maintain implementation only 
methods as public interfaces which we are trying to move away from.

If you are going to black box test the UI classes then you must treat 
them as a black box - that means using an external interface that is 
supported to instrument the testing.  The only supported external 
interface we have is the Toolkit interface and the PG interfaces so you 
must continue to work behind those interfaces and evolve them if testing 
needs more instrumentation or you aren't really testing the UI classes, 
you are testing an approximation of them...

			...jim

On 2/4/13 7:41 AM, Richard Bair wrote:
> Hi Pavel,
>
> Removing StubToolkit _does not imply_ that we have to test with a live toolkit / glass. For example, the PolygonTest wants to test that during synchronization the array of floats it expected to be passed to the peer is the actual set that got passed.
>
>      @Test public void testBounds_oddPointsLength() {
>          final double[] initialPoints = {
>              100, 100, 200, 100, 200, 200, 150, 300
>          };
>
>          final Polygon polygon = new PolygonMock(initialPoints);
>          assertBoundsEqual(box(100, 100, 100, 200), polygon.getBoundsInLocal());
>
>          final ObservableList<Double> polygonPoints = polygon.getPoints();
>          polygonPoints.remove(6);
>
>          assertBoundsEqual(box(100, 100, 100, 100), polygon.getBoundsInLocal());
>      }
>
>
> This can easily be done by using a special subclass of Polygon that, in impl_createPGNode, creates a special subclass of NGPolygon:
>
>      private static final class PolygonMock extends Polygon {
>          public PolygonMock() {
>              super();
>          }
>
>          public PolygonMock(double... points) {
>              super(points);
>          }
>
>          @Override protected PGNode impl_createPGNode() {
>              return new StubPolygon();
>          }
>      }
>
>      private static final class StubPolygon extends NGPolygon {
>          float[] points;
>          @Override public void updatePolygon(float[] points) {
>              this.points = points;
>              super.updatePolygon(points);
>          }
>      }
>
>
> Running this test does not require that we fire up multiple threads or open a glass window etc.
>
> I haven't finished combing through all the usages of StubToolkit, but I'm confident we can remove it without changing the nature of the tests.
>
> Richard
>
> On Feb 4, 2013, at 1:29 AM, Pavel Safrata <pavel.safrata at oracle.com> wrote:
>
>> Hi Richard,
>> I strongly object. I've had this discussion with Kevin a year ago, quoting myself from the old discussion:
>>
>> Scenegraph unit tests are there to test scenegraph. I believe that unit tests should by their definition test single units isolated from the rest of the system, not the system as a whole - for this we should have functional tests developed by QA. Our tests make sure that scenegraph works correctly given that the underlying platform behaves as expected. Including live toolkit/glass in running the tests would IMHO seriously damage purpose of those tests - they wouldn't give any reliable evidence of scenegraph's stability any more. Other important thing is that the code doesn't change over time (so it tests regressions in scenegraph) and also the test can make it behave in more different ways (and test scenegraph robustness).
>>
>> Thanks,
>> Pavel
>>
>> On 2.2.2013 1:11, Richard Bair wrote:
>>> Well, at least that is what I'm hoping to do. The problem is that we have a bunch of unit tests. 20K+. Some of them rely on StubToolkit, some of them rely on the real deal. In the "source cleanup" operation I'm on, when we combine all the "graphics" tests together, we have a problem where these two different types of tests are together and one set will fail to execute depending on whether we launch with the real toolkit or the stub toolkit.
>>>
>>> My plan is to yank out StubToolkit and update all the tests that relied on it to instead work with the real toolkit (quantum and prism). I suspect this is going to be painful.
>>>
>>> Richard
>>
>