phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

CT: PointOnParabolaLinesNode.ts assertion failure #183

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

Probably introduced in TypeScript conversion.

graphing-quadratics : multitouch-fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1678105256042/graphing-quadratics/graphing-quadratics_en.html?continuousTest=%7B%22test%22%3A%5B%22graphing-quadratics%22%2C%22multitouch-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1678105256042%22%2C%22timestamp%22%3A1678108141285%7D&brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=false
Query: brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=false
Uncaught Error: Assertion failed
Error: Assertion failed
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1678105256042/assert/js/assert.js:28:13)
at assert (PointOnParabolaLinesNode.ts:65:18)
at callback (Multilink.ts:111:10)
at listener (TinyEmitter.ts:95:8)
at emit (ReadOnlyProperty.ts:309:22)
at _notifyListeners (ReadOnlyProperty.ts:260:13)
at unguardedSet (ReadOnlyProperty.ts:244:11)
at set (Property.ts:54:10)
at (FocusAndDirectrixModel.ts:102:8)
at listener (TinyEmitter.ts:95:8)
id: Bayes Puppeteer
Snapshot from 3/6/2023, 5:20:56 AM
pixelzoom commented 1 year ago

Here's the relevant code in PointOnParabolaLinesNode.ts, failing at the assertion at line 65, which was added during TypeScript conversion. I'm guessing that this was always a problem, and that quadratic.directrix was being coerced to zero on 71. I'm not sure how a quadratic can be a parabola and not have a directrix.

    // update the lines
    Multilink.multilink( [ quadraticProperty, pointOnParabolaProperty ],
      ( quadratic, pointOnParabola ) => {

        assert && assert( quadratic.isaParabola(), `expected a parabola, quadratic=${quadratic}` );
        assert && assert( quadratic.focus );
65      assert && assert( quadratic.directrix );

        const pointView = modelViewTransform.modelToViewPosition( pointOnParabola );
        const focusView = modelViewTransform.modelToViewPosition( quadratic.focus! );
71      const directrixView = modelViewTransform.modelToViewY( quadratic.directrix! );

        focusLine.setLine( pointView.x, pointView.y, focusView.x, focusView.y );
        directrixLine.setLine( pointView.x, pointView.y, pointView.x, directrixView );
      } );
pixelzoom commented 1 year ago

I could not reproduce this locally using brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=false. I also tried added screens=4, since this code is specific to the "Focus & Directrix" screen.

Unassigning for now, to be investigated when this sim is worked on again.

pixelzoom commented 1 year ago

Fixed in the above commit. quadratic.directrix is defined as public readonly directrix?: number, and may be zero. So the assertion must be:

-        assert && assert( quadratic.directrix );
+        assert && assert( quadratic.directrix !== undefined );