pibizza commented on code in PR #3819:
URL:
https://github.com/apache/incubator-kie-kogito-runtimes/pull/3819#discussion_r1899600009
##########
jbpm/jbpm-flow/src/test/java/org/jbpm/process/core/timer/BusinessCalendarImplTest.java:
##########
@@ -233,8 +233,12 @@ void rollCalendarAfterHolidays() {
List<Integer> weekendDays = Collections.emptyList();
Calendar calendar = Calendar.getInstance();
int currentDayOfYear = calendar.get(Calendar.DAY_OF_YEAR);
+ int maxDaysInYear = calendar.getActualMaximum(Calendar.DAY_OF_YEAR);
BusinessCalendarImpl.rollCalendarAfterHolidays(calendar, holidays,
weekendDays, false);
int expected = currentDayOfYear + holidayLeft + 1;
+ if (expected > maxDaysInYear) {
+ expected -= maxDaysInYear;
+ }
Review Comment:
I understand the patch, but I think the test is wrong. Having an if in a
test to define expected values is kind of a code smell. we should know what to
expect in a test. If I understsand correclty, the issue here is that the test
is not using a fixed day, but instead taking the day we are executing the test.
It is sensible to test the roll of the date, but this means we need two
tests - one near the end of the year and one far from the end of the year. I
suggest modifying the tests according.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
