Copilot commented on code in PR #6912: URL: https://github.com/apache/incubator-kie/pull/6912#discussion_r3860841749
########## drools-ruleunits/drools-ruleunits-dsl/src/test/java/org/drools/ruleunits/dsl/attribute/NoLoopUnit.java: ########## @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.drools.ruleunits.dsl.attribute; + +import java.util.ArrayList; +import java.util.List; + +import org.drools.ruleunits.api.DataSource; +import org.drools.ruleunits.api.DataStore; +import org.drools.ruleunits.dsl.RuleUnitDefinition; +import org.drools.ruleunits.dsl.RulesFactory; +import org.drools.ruleunits.dsl.domain.Person; + +import static org.drools.model.Index.ConstraintType.EQUAL; + +public class NoLoopUnit implements RuleUnitDefinition { + + private final DataStore<Person> persons; + private final List<String> results = new ArrayList<>(); + + public NoLoopUnit() { + this(DataSource.createStore()); + } + + public NoLoopUnit(DataStore<Person> persons) { + this.persons = persons; + } + + public DataStore<Person> getPersons() { + return persons; + } + + public List<String> getResults() { + return results; + } + + @Override + public void defineRules(RulesFactory rulesFactory) { + rulesFactory.rule("NoLoopRule") + .noLoop() + .on(persons) + .filter("name", p -> p.getName(), EQUAL, "Mario") + .executeOnDataStore(persons, (ps, p) -> { + results.add("fired-" + p.getAge()); + p.setAge(p.getAge() + 1); + ps.update(p, "age"); Review Comment: This does not actually exercise `noLoop`: the pattern reacts only to `name`, while the consequence publishes an `age`-only update, so property reactivity prevents this rule from being reactivated even if the attribute is dropped. Publish a `name` update (or make the constraint react to `age`) so removing `noLoop` would cause another firing. ########## drools-ruleunits/drools-ruleunits-dsl/src/test/java/org/drools/ruleunits/dsl/attribute/RuleUnitsAttributesTest.java: ########## @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.drools.ruleunits.dsl.attribute; + +import org.drools.core.common.ReteEvaluator; +import org.drools.ruleunits.api.RuleUnitInstance; +import org.drools.ruleunits.api.RuleUnitProvider; +import org.drools.ruleunits.dsl.domain.Person; +import org.drools.ruleunits.impl.AbstractRuleUnitInstance; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class RuleUnitsAttributesTest { + + @Test + public void timer() { + // incubator-kie#6911: RuleUnit DSL does not apply RuleConfig clock type, so pseudo clock cannot be used here. + // When fixed, use pseudo clock to advance time and verify the timer rule fires after the delay. + TimerUnit unit = new TimerUnit(); + unit.getStrings().add("Hello Timer"); + + try (RuleUnitInstance<TimerUnit> unitInstance = RuleUnitProvider.get().createRuleUnitInstance(unit)) { + assertThat(unitInstance.fire()).isZero(); + assertThat(unit.getResults()).isEmpty(); + } + } + + @Test + public void calendars() { + // incubator-kie#6911: RuleUnit DSL does not apply RuleConfig clock type, so pseudo clock cannot be used here. + // When fixed, use pseudo clock to also test calendar + timer combination. + CalendarsUnit unit = new CalendarsUnit(); + + try (RuleUnitInstance<CalendarsUnit> unitInstance = RuleUnitProvider.get().createRuleUnitInstance(unit)) { + ReteEvaluator evaluator = (ReteEvaluator) ((AbstractRuleUnitInstance) unitInstance).getEvaluator(); + evaluator.getCalendars().set("myCalendar", timestamp -> true); + + unit.getStrings().add("Hello World"); + assertThat(unitInstance.fire()).isEqualTo(1); + assertThat(unit.getResults()).containsExactly("calendar fired"); + } + } + + @Test + public void dateEffectiveAndDateExpires() { Review Comment: Both fixture rules use a `dateEffective` value in the past, so this test still passes if `DATE_EFFECTIVE` is never propagated; only expiration is meaningfully exercised. Add a third rule whose effective date is in the future and expiration is later still, then verify that it does not fire alongside the active rule. ########## drools-ruleunits/drools-ruleunits-dsl/src/test/java/org/drools/ruleunits/dsl/attribute/RuleUnitsAttributesTest.java: ########## @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.drools.ruleunits.dsl.attribute; + +import org.drools.core.common.ReteEvaluator; +import org.drools.ruleunits.api.RuleUnitInstance; +import org.drools.ruleunits.api.RuleUnitProvider; +import org.drools.ruleunits.dsl.domain.Person; +import org.drools.ruleunits.impl.AbstractRuleUnitInstance; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class RuleUnitsAttributesTest { + + @Test + public void timer() { + // incubator-kie#6911: RuleUnit DSL does not apply RuleConfig clock type, so pseudo clock cannot be used here. + // When fixed, use pseudo clock to advance time and verify the timer rule fires after the delay. + TimerUnit unit = new TimerUnit(); + unit.getStrings().add("Hello Timer"); + + try (RuleUnitInstance<TimerUnit> unitInstance = RuleUnitProvider.get().createRuleUnitInstance(unit)) { + assertThat(unitInstance.fire()).isZero(); + assertThat(unit.getResults()).isEmpty(); + } + } + + @Test + public void calendars() { + // incubator-kie#6911: RuleUnit DSL does not apply RuleConfig clock type, so pseudo clock cannot be used here. + // When fixed, use pseudo clock to also test calendar + timer combination. + CalendarsUnit unit = new CalendarsUnit(); + + try (RuleUnitInstance<CalendarsUnit> unitInstance = RuleUnitProvider.get().createRuleUnitInstance(unit)) { + ReteEvaluator evaluator = (ReteEvaluator) ((AbstractRuleUnitInstance) unitInstance).getEvaluator(); + evaluator.getCalendars().set("myCalendar", timestamp -> true); + + unit.getStrings().add("Hello World"); + assertThat(unitInstance.fire()).isEqualTo(1); + assertThat(unit.getResults()).containsExactly("calendar fired"); Review Comment: An always-true calendar produces exactly the same result when the `calendars` attribute is omitted, so this test cannot detect a broken mapping. Register an always-false calendar and assert zero firings/an empty result to verify that the rule is actually gated by the configured calendar. -- 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]
