Progress reporting for QuickCheck seems to report progress over 100%

[michaelpj]

I'm not really sure what's going on here, but I've seen progress reports from QuickCheck go over 100%. These are for tests where I've set MaxSuccesses to a higher number, so initially I thought we simply weren't dividing by the right thing. But the code seems to be doing the right thing so I'm not sure what's happening.

cc @coot

[coot]

🤔 I have never observed that with QuickCheck itself in ghci, have you?

[michaelpj]

I don't think this is anything to do with QuickCheck - that just reports the number of tests, which seems fine. I think it must be something about how we're converting that to a percentage...

[Bodigrim]

Maybe tasty way to parse QuickCheck messages is not entirely correct. Any chance that the tests in question have high discard rate?

[BebeSparkelSparkel]

Example of this problem

All
  Properties
    lowlevel
      input-output
        t_write_read:         384% 

when using withMaxSuccess 1000 ...

[BebeSparkelSparkel]

Error in this line

tasty/quickcheck/Test/Tasty/QuickCheck.hs

Line 267 in dcbf320

(\progressText -> yieldProgress emptyProgress { progressPercent = parseProgress progressText })

[Bodigrim]

@BebeSparkelSparkel any idea what's exactly wrong with parseProgress?

[BebeSparkelSparkel]

@Bodigrim Nothing wrong with the parser. QuickCheck prints the number of tests run and not the % done. Also, the number of tests to run cannot be gotten with out running at least one test because withMaxSuccess does not modify the state nor args but rather the returned intermediate result that has the field maybeNumTests.

I was thinking that an IORef could be added that holds the number of tests to run and then mapResult could be used with unsafePerformIO to update the number after one tests has been run.

I also added this issue since QuickCheck seems to not be a primary test framework but rather an add in tool. nick8325/quickcheck#399

[Bodigrim]

@BebeSparkelSparkel I'm afraid it's hard for me to grasp. Can you possibly come up with a minimal standalone reproducer?

[Bodigrim]

OK, so the problem is that there are two ways to set number of tests to execute.

One is to use QuickCheckTests on TestTree level, e. g., localOption (QuickCheckTests 1000). This is a native tasty-quickcheck mechanism, which progress reporting knows about and handles well.

But there is another way to set number of tests: apply withMaxSuccess on Property level. This way it remains hidden from tasty, which continues to think that there are only 100 tests, thus misreporting progress.

@BebeSparkelSparkel is my understanding correct?

[BebeSparkelSparkel]

Yes. I really don't like how withMaxSuccess accomplishes this.

[MaximilianAlgehed]

You can always add a callback that uses an IORef to track the changes to Result that are made by withMaxSuccess and friends.

[Bodigrim]

@michaelpj could you please check whether HEAD fixes the issues you observed?

[michaelpj]

Yep, that fixes it! Thanks @BebeSparkelSparkel