pibizza commented on code in PR #3769:
URL:
https://github.com/apache/incubator-kie-kogito-runtimes/pull/3769#discussion_r1839823016
##########
jbpm/jbpm-flow/src/test/java/org/jbpm/process/core/timer/BusinessCalendarImplTest.java:
##########
@@ -433,4 +459,36 @@ public long advanceTime(long amount, TimeUnit unit) {
}
}
+
+ private static Stream<Arguments> getValidCalendarProperties() {
+
+ return Stream.of(
+ Arguments.of(Map.of(), Map.of(WEEKEND_DAYS, List.of(1, 7),
DAYS_PER_WEEK, 5, START_HOUR, 9, END_HOUR, 17, HOURS_PER_DAY, 8)),
+ Arguments.of(Map.of(WEEKEND_DAYS, "1, 2, 3", HOURS_PER_DAY,
"5"),
+ Map.of(WEEKEND_DAYS, List.of(1, 2, 3), DAYS_PER_WEEK,
4, START_HOUR, 9, END_HOUR, 14, HOURS_PER_DAY, 5)),
+ Arguments.of(Map.of(START_HOUR, "10", HOURS_PER_DAY, "5"),
+ Map.of(WEEKEND_DAYS, List.of(1, 6), DAYS_PER_WEEK, 5,
START_HOUR, 10, END_HOUR, 15, HOURS_PER_DAY, 5)),
+ Arguments.of(Map.of(END_HOUR, "11"),
+ Map.of(WEEKEND_DAYS, List.of(1, 6), DAYS_PER_WEEK, 5,
START_HOUR, 9, END_HOUR, 11, HOURS_PER_DAY, 2)),
+ Arguments.of(Map.of(START_HOUR, "10", END_HOUR, "16"),
+ Map.of(WEEKEND_DAYS, List.of(1, 6), DAYS_PER_WEEK, 5,
START_HOUR, 10, END_HOUR, 16, HOURS_PER_DAY, 6)));
+ }
+
+ private void assertCalendarProperties(BusinessCalendarImpl
businessCalendar, Map<String, Object> expectedValuesMap) throws
NoSuchFieldException, IllegalAccessException {
+ Field daysPerWeekField =
BusinessCalendarImpl.class.getDeclaredField("daysPerWeek");
+ daysPerWeekField.setAccessible(true);
+ Field startHourField =
BusinessCalendarImpl.class.getDeclaredField("startHour");
+ startHourField.setAccessible(true);
+ Field endHourField =
BusinessCalendarImpl.class.getDeclaredField("endHour");
+ endHourField.setAccessible(true);
+ Field hoursInDayField =
BusinessCalendarImpl.class.getDeclaredField("hoursInDay");
+ hoursInDayField.setAccessible(true);
+ Field weekendDaysField =
BusinessCalendarImpl.class.getDeclaredField("weekendDays");
+ weekendDaysField.setAccessible(true);
+
+ assertEquals(expectedValuesMap.get(DAYS_PER_WEEK),
daysPerWeekField.get(businessCalendar));
Review Comment:
Please do not use assertEquals. Use assertj assertions instead.
##########
jbpm/jbpm-flow/src/test/java/org/jbpm/process/core/timer/BusinessCalendarImplTest.java:
##########
@@ -368,6 +382,18 @@ public void testCalculateMinutesPassingWeekend() {
assertThat(formatDate("yyyy-MM-dd HH:mm:ss",
result)).isEqualTo(expectedDate);
}
+ @ParameterizedTest
+ @MethodSource("getValidCalendarProperties")
+ public void testValidation(Map<String, Object> propertyMap, Map<String,
Object> expectedValuesMap) throws NoSuchFieldException, IllegalAccessException {
+ Properties businessCalendarProperties = new Properties();
+ businessCalendarProperties.putAll(propertyMap);
+ List<BusinessCalendarImpl> businessCalendarList = new ArrayList<>();
+ assertDoesNotThrow(() -> {
+ businessCalendarList.add(new
BusinessCalendarImpl(businessCalendarProperties));
+ });
+ assertCalendarProperties(businessCalendarList.get(0),
expectedValuesMap);
Review Comment:
The method hides what are your intentions. Given that it is used only here,
it is easier to inline the whole method.
##########
jbpm/jbpm-flow/src/test/java/org/jbpm/process/core/timer/BusinessCalendarImplTest.java:
##########
@@ -368,6 +382,18 @@ public void testCalculateMinutesPassingWeekend() {
assertThat(formatDate("yyyy-MM-dd HH:mm:ss",
result)).isEqualTo(expectedDate);
}
+ @ParameterizedTest
+ @MethodSource("getValidCalendarProperties")
+ public void testValidation(Map<String, Object> propertyMap, Map<String,
Object> expectedValuesMap) throws NoSuchFieldException, IllegalAccessException {
+ Properties businessCalendarProperties = new Properties();
+ businessCalendarProperties.putAll(propertyMap);
+ List<BusinessCalendarImpl> businessCalendarList = new ArrayList<>();
+ assertDoesNotThrow(() -> {
+ businessCalendarList.add(new
BusinessCalendarImpl(businessCalendarProperties));
+ });
Review Comment:
You are testing that you can create the object in the proper way. Remove the
adding to the list, this is not part of the test at all.
##########
jbpm/jbpm-flow/src/test/java/org/jbpm/process/core/timer/BusinessCalendarImplTest.java:
##########
@@ -433,4 +459,36 @@ public long advanceTime(long amount, TimeUnit unit) {
}
}
+
+ private static Stream<Arguments> getValidCalendarProperties() {
+
+ return Stream.of(
+ Arguments.of(Map.of(), Map.of(WEEKEND_DAYS, List.of(1, 7),
DAYS_PER_WEEK, 5, START_HOUR, 9, END_HOUR, 17, HOURS_PER_DAY, 8)),
+ Arguments.of(Map.of(WEEKEND_DAYS, "1, 2, 3", HOURS_PER_DAY,
"5"),
+ Map.of(WEEKEND_DAYS, List.of(1, 2, 3), DAYS_PER_WEEK,
4, START_HOUR, 9, END_HOUR, 14, HOURS_PER_DAY, 5)),
+ Arguments.of(Map.of(START_HOUR, "10", HOURS_PER_DAY, "5"),
+ Map.of(WEEKEND_DAYS, List.of(1, 6), DAYS_PER_WEEK, 5,
START_HOUR, 10, END_HOUR, 15, HOURS_PER_DAY, 5)),
+ Arguments.of(Map.of(END_HOUR, "11"),
+ Map.of(WEEKEND_DAYS, List.of(1, 6), DAYS_PER_WEEK, 5,
START_HOUR, 9, END_HOUR, 11, HOURS_PER_DAY, 2)),
+ Arguments.of(Map.of(START_HOUR, "10", END_HOUR, "16"),
+ Map.of(WEEKEND_DAYS, List.of(1, 6), DAYS_PER_WEEK, 5,
START_HOUR, 10, END_HOUR, 16, HOURS_PER_DAY, 6)));
+ }
+
+ private void assertCalendarProperties(BusinessCalendarImpl
businessCalendar, Map<String, Object> expectedValuesMap) throws
NoSuchFieldException, IllegalAccessException {
+ Field daysPerWeekField =
BusinessCalendarImpl.class.getDeclaredField("daysPerWeek");
+ daysPerWeekField.setAccessible(true);
Review Comment:
these two lines are a good candidate for a small helper method that takes as
input a string fieldName and returns a field
At that point you can do a test like
assertThat(field("startHour").get(businessCalendar).isEqualTo(startHour).
--
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]
