Kwxm/test/extra integer property tests #7134

kwxm · 2025-06-05T19:10:46Z

Fixes 7068.

This GHC bug and the ones revealed by our property tests for expModInteger made me a bit nervous about the integer arthimetic and comparison builtins, which are some of the earliest ones and were a bit lacking in tests. I've added some property tests that might help to make us a bit more confident.

kwxm · 2025-06-05T19:12:32Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/ExpModIntegerProperties.hs

This is ExpModInteger.hs, but I think I used mv instead of git mv and GitHub doesn't realise that it's the same file (tidied up and reformatted a bit)..

The tests here need to be quite careful about thier inputs, so I've used arbitrary explicitly and filtered its values. The other tests use more standard generators.

github-actions · 2025-06-05T19:14:41Z

PR Preview Action v1.6.1
🚀 View preview at https://IntersectMBO.github.io/plutus/pr-preview/pr-7134/
Built to branch `gh-pages` at 2025-06-20 22:03 UTC. Preview will be ready when the GitHub Pages deployment is complete.

kwxm · 2025-06-05T19:15:55Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/Common.hs

+import PlutusCore.MkPlc (builtin, mkIterAppNoAnn, mkTyBuiltin, tyInst)
+
+import Evaluation.Builtins.Common
+


More utility functions for creating PLC terms. I'm probably reinventing the wheel yet again here.

kwxm · 2025-06-05T19:17:13Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/DivModProperties.hs

+test_integer_div_mod_properties :: TestTree
+test_integer_div_mod_properties =
+  testGroup "Property tests for divideInteger and modInteger"
+    [ testProp "divideInteger _ 0 always fails" prop_div_0_fails


I wasn't sure whether to put the test descriptions here or at the top of the tests themselves. This seems a bit more compact, but maybe putting the descriptions in the tests would have been better from a documentation point of view.

I'd say it's ok either way.

kwxm · 2025-06-05T19:18:28Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/QuotRemProperties.hs

The tests here are very similar to those for divideInteger and modInteger, but not identical because there are significant differences in behaviour for negative inputs.

kwxm · 2025-06-05T19:21:58Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/RingProperties.hs

+{-# LANGUAGE TypeApplications  #-}
+{-# LANGUAGE ViewPatterns      #-}
+
+{- | Property tests for the `addInteger`, `subtractInteger`, and `multiplyInteger` builtins -}


The tests aren't very exciting, but I felt obliged to add something.

effectfully

Sorry for the overly late review.

Great tests, but I think coverage is unfortunate due to QuickCheck being silly.

effectfully · 2025-06-19T01:01:19Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/DivModProperties.hs

+test_integer_div_mod_properties :: TestTree
+test_integer_div_mod_properties =
+  testGroup "Property tests for divideInteger and modInteger"
+    [ testProp "divideInteger _ 0 always fails" prop_div_0_fails


I'd say it's ok either way.

effectfully · 2025-06-19T01:07:43Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/DivModProperties.hs

+testProp s p = testProperty s $ withMaxSuccess numberOfTests p
+
+-- `divideInteger _ 0` always fails.
+prop_div_0_fails :: Integer -> Property


Are you aware that QuickCheck only generates Integers within the [-size, size] (or something) range with size typically being at most 100? E.g. sample sets the size to 20 or 30 or something:

-- >>> import Test.QuickCheck -- >>> sample (arbitrary :: Gen [[Integer]]) -- [] -- [] -- [[-1],[],[-1]] -- [[]] -- [[-5],[4]] -- [[7,5,8,4],[3,-6,-10,9,2,-3,-10,7,-5],[-8,8,0],[0,2,-2,-10],[-6,-8,-1,-3,9,-5,-9,2,-1,-1],[-4,1,-2,9,-6,9,-9,8,-8,6]] -- [[0],[-6,-1,12,9,-6,-5,-9,-5,-12,7],[5,12,-6,4],[8,-6,3,-3,12,-10,-12,12,-2],[-1,12],[-5],[5,-9,0],[2,-7,11,-12,12],[-10,-3,-7,12,11,11,10,5,0,2],[11,-5,2,-5,12,-8,-6,0,8],[-7],[4,3,9,-12,6,11,-8,1,8,-10]] -- [[-13,-5,-2,11,2,2,13],[11,10,-5,-6,-12,-1,12,-7,9],[-8,-9,-12,-8,6,14,5,-14,4,-4,-9],[13,10,-10,-14,9,-2,-10,-9,11,10,5,14],[14,1],[4,-11,13,-7,9,-9,4,11,-14],[-8,-3,-12,6],[12,-14,0,2,0,10,9,-10,-12,-1,5,7,2],[4,3,2,8,-13],[-7,-5,-4,-10,-1,-6,-2,-9,-1],[8,8,-5,-10,0,7,-8,7,-1],[14,-7,-10,8,2,10,3,-9,-1,-7,1,-11]] -- [[-3,3],[-3,15,-1,-7,-13,14,-1,-13,15,0,-12,11],[13,-8,-13],[-9,-1,5,-10,-3,7,13,7,-12,13,0,11,8,-7,-15,11],[-16],[10,3,15,3,-12,-2,11,8,7,-11],[-3,-9,-12,-16],[],[-13,4,0,-9],[7,12,4,-15,-15,-9,-10,5,-7],[-13,14,12,-6,13,-10,2,7,-12,10,3,12,-10],[-12,-11,-12,6,6,-14,-11],[-10,16,-10,-5,-8,13,2,11],[-15,11]] -- [[3,-3,-5,-10],[],[7,5,7,1,18,-6,1,-12,-2,13],[16,8,-5,-9,2,-10,-1,15,-3,14,-4,-11],[12,7,-9,-14,14,15,-15,-5,4,11,-4,12,-2,6,18,4],[16,-12,0,-11,-15,17,-6,6,3,-7]] -- [[20,14,-7,7,-6,17,6,-20,-14,5,-9,3],[2,-6,-10,14,0,17,-6,-2],[-15,-15,-20,9,-20,3,-4,7,9,-7,-8,-7,-11,11,18,14,6,-12,-6,17],[12,-14,7,14,-6,-5],[13,5,-7,0,0,-17,8,14,15,4],[19,-20,10,1,-2,17,2,10,18,-3,17,-10,-18,1],[9,-9,8,-11,12,-19,-5],[14,11,19,14,14,-7,-11,-18,1,0,-16,-7],[-13,-20],[13,-13,-8,-15,18],[],[2,17,-15,-10,-12,-18,9],[4,-18,-16,3,14,-11,9,-20,-16,0,16],[5,3,0,4,5,6,0,-9,-15,-6,-7,8,12,-9]]

I'd suggest using AsArbitraryBuiltin Integer (feel free to alias it to something more palatable).

Are you aware that QuickCheck only generates Integers within the [-size, size] (or something) range with size typically being at most 100?

Ah, I'd forgotten about that. I seem to remember that what sample does is very misleading because it produces values from a much more restricted range than you usually get. I'll go away and remind myself about this stuff.

@effectfully I've updated the tests to use the generator for AsArbitraryBuiltin Integer (which meant that I had to add some extra deriving clauses for AsArbitraryBuiltin). Thanks for the suggestion.

Unisay · 2025-06-20T13:30:36Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Definition.hs

+        , test_integer_div_mod_properties
+        , test_integer_quot_rem_properties
+        , test_integer_order_properties
+        , test_integer_ring_properties


makes sense to rename the test below this one for its name to be consistent:
test_expModInteger_properties -> test_integer_exp_mod_properties

kwxm · 2025-06-20T22:06:38Z

plutus-core/testlib/PlutusCore/Generators/QuickCheck/Builtin.hs

@@ -278,7 +278,7 @@ instance ArbitraryBuiltin BLS12_381.Pairing.MlResult where
 -- the 'Arbitrary' class and the logic for handling elements from 'ArbitraryBuiltin'.
 newtype AsArbitraryBuiltin a = AsArbitraryBuiltin
    { unAsArbitraryBuiltin :: a
-    } deriving newtype (Show)
+    } deriving newtype (Show, Eq, Ord, Num)


These are needed to make things like Positive and NonZero work.

kwxm · 2025-06-20T22:07:36Z

plutus-core/untyped-plutus-core/testlib/Evaluation/Builtins/Integer/Common.hs

+-- for Integers because that only produces values in [-99..99].  Here are some
+-- utilities to make it easier to use the better generator for
+-- AsArbitraryBuiltin Integer.
+type BigInteger = AsArbitraryBuiltin Integer


They're not always big, but I couldn't think of a better name.

kwxm added 24 commits May 30, 2025 10:16

Extra property tests for integer builtins

3fedc1f

Formatting

dda65fa

New files

e21ee78

WIP

8eb83d3

Missing file

1258a59

WIP

6ca0ed9

WIP

09f2df1

WIP

ff3d023

WIP

d7da44e

WIP

26cc11a

WIP

0007cec

WIP

44d991e

WIP

43418c6

WIP

9d3efb7

Remove renamed file

5cb109b

WIP

2cf2ee2

WIP

dbca1e9

WIP

5852366

Tidying up

df3f3e6

More comments

1007e59

More comments

93429af

More comments

1f980ad

Move file to try to make git see that it's renamed

5197e25

Move file back

9744dae

kwxm added Test Builtins No Changelog Required Add this to skip the Changelog Check labels Jun 5, 2025

kwxm temporarily deployed to github-pages June 5, 2025 19:10 — with GitHub Actions Inactive

kwxm requested review from Unisay and effectfully June 5, 2025 19:10

kwxm commented Jun 5, 2025

View reviewed changes

Oops: duplicated code

5232cf4

kwxm temporarily deployed to github-pages June 5, 2025 19:21 — with GitHub Actions Inactive

kwxm commented Jun 5, 2025

View reviewed changes

effectfully approved these changes Jun 19, 2025

View reviewed changes

Unisay reviewed Jun 20, 2025

View reviewed changes

Address PR comments

343d638

kwxm temporarily deployed to github-pages June 20, 2025 22:00 — with GitHub Actions Inactive

kwxm commented Jun 20, 2025

View reviewed changes

		import PlutusCore.MkPlc (builtin, mkIterAppNoAnn, mkTyBuiltin, tyInst)

		import Evaluation.Builtins.Common

Kwxm/test/extra integer property tests #7134

Are you sure you want to change the base?

Kwxm/test/extra integer property tests #7134

Uh oh!

Conversation

kwxm commented Jun 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2025-06-20 22:03 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

effectfully left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 5, 2025 •

edited

Loading

Built to branch `gh-pages` at 2025-06-20 22:03 UTC.
Preview will be ready when the GitHub Pages deployment is complete.