Just skimmed the PR, I'm sure the author knows more than I - but why hard code a date at all? Why not do something like `today + 1 year`?
We experienced several of those over the years, and generally it was the test that was wrong, not the code it was testing.
For example, this simplified test hits several of those pitfalls:
var expected = start.AddMonths(1);
var actual = start.ToLocal().AddMonths(1).ToUtc();
Assert(expected == actual);Always include some randomness in test values.
- Test 1 -> set data_1 with value 1
- Test 1 -> `do some magic`
- Test 1 -> assert value 1 + magic = expected value
- Test 2 -> set data_1 with value 2
But this can fail if `do some magic` is slow and Test 2 starts before Test 1 asserts.
So I can either stop parallelism, but in real life parallelism exists, or ensure that each test as random id, just like it would happen in real life.
doing random things to hopefully get a failure is fine if there's an actual purpose to it, but putting random values all over the place in the hopes it reveals a problem in your CI pipeline or something seems like a real weak reason to do it.
then it actually is a huge success
because it found a bug you overlooked in both impl. and tests
at least iff we speak about unit tests
The whole concept of allowing a flaky unit test to exist is wild and dangerous to me. It makes a culture of ignoring real failures in what, should be, deterministic code.
So, yes, logging the inputs is extremely important. So is minimizing any IO dependency in your tests.
But then that runs against another important rule, that integration tests should test the entire system, IO included. So, your error handling must always log very clearly the cause of any IO error it finds.
If this isn't a joke, I'd be very interested in the reasoning behind that statement, and whether or not there are some qualifications on when it applies.
If you test math_add(1,2) and it returns 3, you don't know if the code does `return 3` or `return x+y`.
It seems I might need to revise my view.
jitter = random(5)
assertEqual(3 + jitter, math_add(1, 2 + jitter))
Nowadays I think this would be done with fuzzing/constraint tests, where you define "this relation must hold true" in a more structured way so the framework can choose random values, test more at once, and give better failure messages.
Damn, must be why only white hair is growing on my head now.
>Nowadays I think this would be done with fuzzing/constraint tests, where you define "this relation must hold true" in a more structured way so the framework can choose random values, test more at once, and give better failure messages.
So the concept of random is still there but expressed differently ? (= Am I partially right ?)
Here's an example with a python library: https://hypothesis.readthedocs.io/en/latest/tutorial/introdu...
The strategy "st.lists(st.integers())" generates a random list of integers that get passed into the test function.
And also this page says by default tests would be run (up to) 100 times: https://hypothesis.readthedocs.io/en/latest/tutorial/setting...
So I'm thinking... (not tested)
@given(st.integers(), st.integers())
def test_math_add(a, b):
assert a + b == math_add(a, b)
so if you generate test data randomly you have a higher chance of "accidentally" running into overlooked edge cases
you could say there is a "adding more random -> cost" ladder like
- no randomness, no cost, nothing gained
- a bit of randomness, very small cost, very rarely beneficial (<- doable in unit tests)
- (limited) prop testing, high cost (test runs multiple times with many random values), decent chance to find incorrect edge cases (<- can be barely doable in unit tests, if limited enough, often feature gates as too expensive)
- (full) prop testing/fuzzing, very very high cost, very high chance incorrect edge cases are found IFF the domain isn't too large (<- a full test run might need days to complete)
people often take flaky test way less serious then they should
I had multiple bigger production issues which had been caught by tests >1 month before they happened in production, but where written off as flaky tests (ironically this was also not related to any random test data but more load/race condition related things which failed when too many tests which created full separate tenants for isolation happened to run at the same time).
And in some CI environments flaky test are too painful, so using "actual" random data isn't viable and a fixed seed has to be used on CI (that is if you can, because too much libs/tools/etc. do not allow that). At least for "merge approval" runs. That many CI systems suck badly the moment you project and team size isn't around the size of a toy project doesn't help either.
So don't do that. That's bad practice. The test has failed for a reason and that needs to be handled.
Some tests can be at the mercy of details that are hard to control, e.g. thread scheduling, thermal-based CPU throttling, or memory pressure from other activity on the system
A test should communicate its reason for testing the subject, and when an input is generated or random, it clearly communicates that this test doesn't care about the specific _value_ of that input, it's focussed on something else.
This has other beneficial effects on test suites, especially as they change over the lifetime of their subjects:
* keeping test data isolated, avoiding coupling across tests * avoiding magic strings * and as mentioned in this thread, any "flakiness" is probably a signal of an edge-case that should be handled deterministically and * it's more fun [1]