Fix flaky test TimeAndDateTest.testPastDate#1216
Fix flaky test TimeAndDateTest.testPastDate#1216asolntsev merged 2 commits intodatafaker-net:mainfrom asolntsev:refactoring/add-details-to-thrown-errors
TimeAndDateTest.testPastDate#1216Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1216 +/- ##
============================================
- Coverage 92.35% 92.05% -0.30%
- Complexity 2821 2963 +142
============================================
Files 292 304 +12
Lines 5609 5855 +246
Branches 599 624 +25
============================================
+ Hits 5180 5390 +210
- Misses 275 305 +30
- Partials 154 160 +6 ☔ View full report in Codecov by Sentry. |
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Failed to read path " + s, e); |
There was a problem hiding this comment.
| throw new RuntimeException("Failed to read path " + s, e); | |
| throw new RuntimeException("Failed to read path '" + s + "'", e); |
sometimes when string is empty such output with quotes could save time during debug
another version with formatting is also ok
There was a problem hiding this comment.
Good point, fixed.
| .generate()) | ||
| .isInstanceOf(IllegalArgumentException.class) | ||
| .hasMessage("Max length must be not less than min length and not negative"); | ||
| .hasMessage("Max length (5) must be not less than min length (10) and not negative"); |
There was a problem hiding this comment.
wouldn't it be better to extract it into vars?
There was a problem hiding this comment.
I would say it makes this test more complex.
These values are pretty near and well-readable. After introducing variables, it will be harder to read this test.
snuyanzin
left a comment
snuyanzin
left a comment
There was a problem hiding this comment.
LGTM
I have a couple of minor comments
it happened when 1. random TimeUnit was chosen NANOSECOND, but 2. random "atMost" (in milliseconds) was chosen lesser than 1000000 (it's less than 1 millisecond), so upperBoundMillis was equal to 1. Sample stack trace: ``` java.lang.IllegalArgumentException: bound must be positive: -1 at net.datafaker.service.RandomService.nextLong(RandomService.java:51) at net.datafaker.providers.base.TimeAndDate.past(TimeAndDate.java:182) at net.datafaker.providers.base.TimeAndDate.past(TimeAndDate.java:131) at net.datafaker.providers.base.TimeAndDate.past(TimeAndDate.java:119) at net.datafaker.providers.base.TimeAndDateTest.testPastDate(TimeAndDateTest.java:66) ```
3 participants
Also, add details to thrown errors.