StubToolkit must go!

[Richard Bair]

All testing is only an approximation. Almost all of our unit tests are not black-box testing but white-box testing. SQE tests are presently black-box tests and they exercise the whole platform. We can run 20,000 unit tests in about the time it takes to run 50-100 SQE tests. There are other tradeoffs as well. For example the unit tests, when they fail, it is usually pretty easy to know what went wrong because you aren't testing very much of the platform -- only a very specific aspect of it (that is the inherent benefit of mocks). Black box testing on the other hand will find errors in places that you hadn't thought off, since it is testing more. Then again, when there is a failure, it is more work to diagnose where the failure occurred. And to do stringent testing takes many, many hours as often tests need to be fired up in separate VM's and will produce different results on different platforms, and you lose control of your OS while those tests run since it is actually driving the mouse / keyboard.

So while there are downsides to using mocks like this, there are also upsides. I think you need both kinds of tests in the end.

But this is really interim anyway. There is nothing in the NG nodes that dictates that we can't use them fully headless in a unit-test fashion. The problem is that javafx-sg-prism depends on javafx-ui-common, and we can't make javafx-ui-common (and all its tests) depend on javafx-sg-prism. In the "new world" I'm setting up, both of these projects (and a few others) will be merged into the "Graphics" component (module) and then we can get the tests cleaned up the right way.

Richard

On Feb 4, 2013, at 6:06 PM, Jim Graham <james.graham at oracle.com> wrote:

> 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
>>> 
>>