Skip to content

user larger keys for IntMap tests #788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jwaldmann opened this issue Jul 22, 2021 · 9 comments
Closed

user larger keys for IntMap tests #788

jwaldmann opened this issue Jul 22, 2021 · 9 comments

Comments

@jwaldmann
Copy link
Contributor

Prompted by #787 (comment), I replaced the Arbitrary instance with

instance Arbitrary a => Arbitrary (IntMap a) where
  arbitrary = fmap (fromList . fmap (\(k,v) -> (getLarge k, v))) arbitrary

Good thing: all tests are green. So, no action required. We still might want to replace the instance?

That section of the code has heading Arbitrary, reasonably balanced trees. I think "reasonable" is the wrong goal here - for correctness tests, I want all sorts of data, including "unreasonable". This might be different from performance tests where we might want typical, and that could mean "small keys", as produced by the current instance.

@treeowl
Copy link
Contributor

treeowl commented Jul 22, 2021

I certainly agree that we want extreme trees! Is Large a good way to get that? Or do we need to combine multiple generators? If you think this version is more thorough than what we did before, I'll happily merge.

Quick question: do you have any idea how to up the test count in CI?

@jwaldmann
Copy link
Contributor Author

Is Large a good way to get that?

I have no idea, and no experience. Large looked to me like one standard way to quickly get Ints that use all bits. I found out about it only after I made the ad-hoc definition 2^x * y. Quickcheck's generator https://hackage.haskell.org/package/QuickCheck-2.14.2/docs/src/Test.QuickCheck.Arbitrary.html#arbitrarySizedBoundedIntegral is different (better?) in the sense that it will use all bits (mine would just shift a few-bit pattern). On the other hand, an Int with a long tail of zeroes might be useful for the specific data structures that we want to test.

If you think this version is more thorough than what we did before, I'll happily merge.

"more thorough" is hard to define/measure. One could fuzz the code and see how fast that's detected? (Nice student project ...)

Quick question: do you have any idea how to up the test count in CI?

On the command line, I used stack test --ta '-a 10000'. containers CI uses cabal, I'm sure it has a way to pass options. This seems to work:

cabal test intset-properties --test-option="-a 10000"

The CI script is generated? Then we need to find a way to pass options through the generator?

@treeowl
Copy link
Contributor

treeowl commented Jul 22, 2021

I'll merge now. We can investigate more options later.

@treeowl
Copy link
Contributor

treeowl commented Jul 22, 2021

Oh wait, this isn't a PR. 🤦

@jwaldmann
Copy link
Contributor Author

jwaldmann commented Jul 22, 2021

I can make a PR. For both IntMap and IntSet?

@treeowl
Copy link
Contributor

treeowl commented Jul 22, 2021 via email

@treeowl
Copy link
Contributor

treeowl commented Aug 17, 2021

Why didn't I merge this already?

@jwaldmann
Copy link
Contributor Author

ping

@meooow25
Copy link
Contributor

Arbitrary IntMap tests with large keys now.

instance Arbitrary a => Arbitrary (IntMap a) where
arbitrary = oneof [go arbitrary, go (getLarge <$> arbitrary)]
where
go kgen = fromList <$> listOf ((,) <$> kgen <*> arbitrary)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants