allow more customisation in cardinality, fuzziness and range #46
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
| rangeMin := rand.Intn(100) | ||
| rangeMax := rand.Intn(10000-rangeMin) + rangeMin |
There was a problem hiding this comment.
What's the benefit of using random values here?
There was a problem hiding this comment.
as you know, I prefer to have some fuzziness in tests, to catch potential hidden bugs ;)
the benefit is not from the randomness itself, but for the fact that every cardinality is tested with a different range: the intent is to test that cardinality correctness is not bound to a specific range. we can also have a fixed list of ranges for every cardinality
There was a problem hiding this comment.
I'm not sure this test is actually behaving like that. Without updating the seed the generated values will not change. (Seed docs, it behaves like always using Seed(1))
See for a playground: https://go.dev/play/p/xZNnm-YMp6I
From my understanding without updating the seed (I'm not sure if this happens somewhere, but searching for Seed or rand.Seed in this repo given 0 results) this code behave like using a constant value.
There was a problem hiding this comment.
From my understanding without updating the seed (I'm not sure if this happens somewhere, but searching for
Seedorrand.Seedin this repo given 0 results) this code behave like using a constant value.
yes, I don't update the seed, indeed. but that's not needed: every call to rand.Intn does not generate a constant value, rather, multiple calls with the same seed generate always the same sequence:
Seed values that have the same remainder when divided by 2³¹-1 generate the same pseudo-random sequence
I suppose that once the sequence is completely generated it will start over. I have to look how long is the sequence (int64 size?) but we are far from filling it :)
so basically we test always with the same sequence, indeed, that's why I mentioned that "we can also have a fixed list of ranges for every cardinality" :)
or we can use the current unix timestamp as seed, reducing the number of identical sequences between each tests run
| @@ -159,22 +161,40 @@ And the following config file content: | |||
| - name: AccountID | |||
| value: 123456789012 | |||
| - name: InterfaceID | |||
| cardinality: 10 | |||
| cardinality: | |||
There was a problem hiding this comment.
I think it may make sense to have a assets/templates/example folder where we put these example as they make the Readme harder to read. What do you think?
There was a problem hiding this comment.
Yes, I would add in a separate PR. what about yours for docs?
There was a problem hiding this comment.
ok on doing in a separate PR. I will rebase my doc and update it once this is merged
Co-authored-by: Edoardo Tenani <[email protected]>
Co-authored-by: Edoardo Tenani <[email protected]>
No description provided.