sageserpent-open / americium

Generation of test case data for Scala and Java, in the spirit of QuickCheck. When your test fails, it gives you a minimised failing test case and a way of reproducing the failure immediately.
MIT License
15 stars 1 forks source link

Poor diagnostic when a recipe hash is no longer valid. #72

Closed sageserpent-open closed 6 months ago

sageserpent-open commented 7 months ago

A recipe hash is expected to become invalid when the RocksDB instance holding the recipes is deleted; the RocksDB instance is placed in the Java system temporary directory, so this is expected whenever said directory is cleaned out. On OSX this seems to happen either every day or every user login session, not sure which.

That is expected behaviour; this is why there is provision to pass the full JSON recipe in as a Java property (trials.recipe) to Americium instead of the recipe hash (trials.recipeHash).

What is not acceptable is the poor diagnostic that results if a test is run that uses this recipe:

java.lang.NullPointerException
    at java.base/java.lang.reflect.Array.getLength(Native Method)
    at scala.collection.ArrayOps$.map$extension(ArrayOps.scala:927)
    at com.sageserpent.americium.generation.SupplyToSyntaxSkeletalImplementation.$anonfun$15(SupplyToSyntaxSkeletalImplementation.scala:983)
    at cats.effect.SyncIO.runLoop$1(SyncIO.scala:243)
    at cats.effect.SyncIO.unsafeRunSync(SyncIO.scala:381)
    at com.sageserpent.americium.generation.SupplyToSyntaxSkeletalImplementation.lazyListOfTestIntegrationContexts$$anonfun$1(SupplyToSyntaxSkeletalImplementation.scala:698)
    at scala.collection.immutable.LazyList$.$anonfun$unfold$1(LazyList.scala:1235)
    at scala.collection.immutable.LazyList.scala$collection$immutable$LazyList$$state$lzycompute(LazyList.scala:259)
    at scala.collection.immutable.LazyList.scala$collection$immutable$LazyList$$state(LazyList.scala:252)
    at scala.collection.immutable.LazyList.isEmpty(LazyList.scala:269)
    at scala.collection.immutable.LazyList$LazyIterator.hasNext(LazyList.scala:1250)
    at com.sageserpent.americium.java.CrossApiIterator$$anon$1.hasNext(CrossApiIterator.scala:11)
    at scala.collection.Iterator$$anon$16.hasNext(Iterator.scala:823)
    at scala.collection.Iterator$$anon$9.hasNext(Iterator.scala:583)
    at scala.collection.convert.JavaCollectionWrappers$IteratorWrapper.hasNext(JavaCollectionWrappers.scala:32)
    at java.base/java.util.Spliterators$IteratorSpliterator.tryAdvance(Spliterators.java:1931)
    at java.base/java.util.Spliterators$1Adapter.hasNext(Spliterators.java:681)

That is from running via the JUnit strongly-typed test integration, but I'm pretty sure that other approaches will yield equally unhelpful stack traces leading to the same exception.

What the poor user wants to see is something more like:

com.sageserpent.americium.blah.blah.blah.RecipesStorageIsNotPresent
Error message: Has the directory containing the RocksDB instance for recipes at <path> been deleted?
Please regenerate the recipe by running the test without Java property `trials.recipeHash`, or use `trials.recipe` with the full recipe JSON.

Another case to consider is when the RocksDB instance is present but the recipe hash doesn't associate with a recipe. This can happen when switching back and forth between distinct tests with their own recipe - if after the RocksDB instance is deleted the recipe hash is regenerated for one test, that will recreate the RocksDB instance but the other test's recipe hash will be invalid.

sageserpent-open commented 6 months ago

Added a test to reproduce this in Git commit SHA: 680fb012e20c8ef39731e577e2c0ab8609562362.

The stack trace is slightly different as the code has changed in the meantime, but it is essentially the same problem...

Caused by: java.lang.NullPointerException
    at java.base/java.lang.reflect.Array.getLength(Native Method)
    at scala.collection.ArrayOps$.map$extension(ArrayOps.scala:929)
    at com.sageserpent.americium.storage.RocksDBConnection.recipeFromRecipeHash(RocksDBConnection.scala:144)
    at com.sageserpent.americium.generation.SupplyToSyntaxSkeletalImplementation.$anonfun$shrinkableCases$26(SupplyToSyntaxSkeletalImplementation.scala:916)
    at cats.effect.SyncIO.runLoop$1(SyncIO.scala:243)
    at cats.effect.SyncIO.unsafeRunSync(SyncIO.scala:381)
    at com.sageserpent.americium.generation.SupplyToSyntaxSkeletalImplementation.supplyTo(SupplyToSyntaxSkeletalImplementation.scala:963)
    at com.sageserpent.americium.generation.SupplyToSyntaxSkeletalImplementation.supplyTo$(SupplyToSyntaxSkeletalImplementation.scala:941)
    at com.sageserpent.americium.TrialsImplementation$SupplyToSyntaxImplementation$1.supplyTo(TrialsImplementation.scala:209)
    at com.sageserpent.americium.TrialsSpecInQuarantineDueToUseOfRecipeHashSystemProperty.$anonfun$new$516(TrialsSpec.scala:2516)
sageserpent-open commented 6 months ago

Implemented, went out in release 1.19.1, Git commit SHA: 47abba480dedaaee20398a97ee85a4af141e5d30.