Skip to content
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

Test not in line with actual implementation of DateAndTime.past() #359

Closed
jaapcoomans opened this issue Sep 15, 2022 · 2 comments
Closed

Comments

@jaapcoomans
Copy link
Contributor

Describe the bug
An unreleated test failed for #358 :

Error: 8.168 [ERROR] Tests run: 40, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.29 s <<< FAILURE! - in net.datafaker.service.FakeValuesServiceTest
Error: 8.169 [ERROR] net.datafaker.service.FakeValuesServiceTest.pastDateExpression  Time elapsed: 0.012 s  <<< FAILURE!
java.lang.AssertionError: 

Expecting actual:
  1663204567147L
to be between:
  ]1663204568040L, 1663222568040L[

The value is out of bounds by 893 millis.

To Reproduce
Run net.datafaker.service.FakeValuesServiceTest.pastDateExpression multiple times until it produces a really low value.

Expected behavior
The test passes.

Additional context
A little investigation shows that the expression in the test resolves to the DateAndTime.past(int, TimeUnit) method. When looking at the implementation (and Javadoc) it shows that it takes a 1000 millisecond offset form now. The test does not. Hence this test will be flaky. Given the large range it will fail very sparsely.
I'm not sure what the thought is behind the 1 second offset / slack of the implementation. Should the test be fixed to match the implementation, or should the implementation be fixed to match the test?

And as a side-note: this kind of flaky behavior is obviously a risk that's in the nature of this library. Can we think of a way to more reliably test randomly generated values?

bodiam added a commit that referenced this issue Sep 16, 2022
bodiam added a commit that referenced this issue Sep 16, 2022
@bodiam
Copy link
Contributor

bodiam commented Sep 16, 2022

Fixed in #361.

@bodiam bodiam closed this as completed Sep 16, 2022
@bodiam
Copy link
Contributor

bodiam commented Sep 16, 2022

an we think of a way to more reliably test randomly generated values?

This used a be quite a big issue in the past, but as long as we use things like @RepeatedTest(100) or something like that, it will catch ~99% of the issues. There's maybe other ways how to fix it, happy to hear, but for now, this works reasonably well.

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

No branches or pull requests

2 participants