Add Property fuzzing

[samreid]

We recently discussed that many options aren't exercised during fuzz testing, see https://github.com/phetsims/aqua/issues/136. We can improve fuzzing coverage by fuzzing the Property values directly, without requiring mouse/touch input to trigger them. For instance, we could automatically switch between discrete valid values like so:

    // Near the end of the Property constructor 
    if ( providedOptions && providedOptions.validValues && this.isSettable() ) {
      stepTimer.setInterval( () => {
        this.value = _.sample( providedOptions.validValues )!;
      }, 200 );
    }

A similar approach could be applied for isSettable NumberProperties that specify a Range, and for BooleanProperty values.

Note the guard on isSettable(), this prevents fuzzing DerivedProperties. It's not clear that isSettable() means the Property can be set to any valid value at any time in the sim without creating an inconsistent state, but many Properties (such as combo boxes, or dialog options) can be set at any time. Would we need additional metadata to Property to indicate whether it is safe to fuzz values at any time? Do we want to proceed with this style of testing?

@pixelzoom can you please review and comment?

[pixelzoom]

It's a nice thought, but @samreid already identified the main problem:

It's not clear that isSettable() means the Property can be set to any valid value at any time in the sim without creating an inconsistent state ...

Game state definitely does not take any value at any time; there's a sequence that makes sense of the game's "state machine".

Even if individual Properties don't object, by fuzzing all Properties that have validValues, you'll likely be putting the sim in states that it would never reach in practice, and which it may not be designed for.

Would we need additional metadata to Property to indicate whether it is safe to fuzz values at any time?

Since I don't think this approach is really generally viable, it would be preferable to opt in than to opt out. For example, for the Property associated with the combo box in Geometric Optics:

  this.opticalObjectChoiceProperty = new EnumerationProperty( options.opticalObjectChoices[ 0 ], {
      fuzzTest: true,
      validValues: options.opticalObjectChoices,
      tandem: options.tandem.createTandem( 'opticalObjectChoiceProperty' )
    } );

One interval (200 ms was used in @samreid's example code above) is unlikely to be a good fit for all Properties. For my example in Geometric Optics, something like 3-5 seconds would probably be a better fit -- we want to significantly test each scene before moving to another scene.

Do we want to proceed with this style of testing?

My gut feeling is no. Besides the problems noted above, this also doesn't address exercising the UI.

[samreid]

I tried adding

    // Near the end of the Property constructor 
    if ( providedOptions && providedOptions.validValues && this.isSettable() ) {
      stepTimer.setInterval( () => {
        this.value = _.sample( providedOptions.validValues )!;
      }, 200 );
    }

and testing Geometric Optics, and it seemed like it was testing reasonable things. But it didn't seem to add a lot of value beyond input fuzzing, and had incomplete coverage. For example, booleans weren't toggled on and off. Also, having to visit sites and opt-in to this level of testing seems like additional burden. Perhaps we should close this issue until a stronger need arises.

[samreid]

If we implement this, we could follow a strategy like KeyboardFuzzer, and get control over the amount of Properties changed in one frame.

[samreid]

@zepumph and I discussed that adding Property-based fuzzing on an opt-in basis could help us get much better coverage in our automated testing.

[samreid]

I'm having second thoughts about this issue:

• In Center and Variability, one of the main controls that should be tested in automated testing is the "maximum number of kicks" combo box which is in the preferences dialog. However, changing that value clears all the data points, so it shouldn't be exercised too often.
• Defining a language for "how often should this Property" be changed seems like it would add a lot of complexity.
• Would we want to auto-trigger Emitters too?
• A developer can already tap into ?fuzz in a sim-specific way like so:

if (assert && queryParameters.fuzz){
  setInterval(myProperty.value = sampleRandom(myProperty.validValues);
}

That is, we do not need a framework or axon options to make sim-specific fuzzing work in this way. However, I do not think anyone would want to see that additional bloat as a long-term addition to their code. Nor would we want to see and maintain property options like fuzzMe: FuzzStrategy.changeValueEveryNSeconds(10, validValues).

So I recommend this issue be closed again. @zepumph please reopen for any reason.