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

[DROOLS-4636] Update Scenario Cheatsheet to mention MVEL and Background #1277

Merged
merged 9 commits into from
Jan 22, 2020

Conversation

danielezonca
Copy link
Member

Copy link
Member

@dupliaka dupliaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, reviewed with some comments

@@ -189,3 +189,6 @@ validationSucceed=Validation Succeed

backgroundTabTitle=Background
backgroundErrorNotification=There are errors in Background data, please have a look
ruleCheatSheet23=To specify a MVEL expression (or invoke a Java method) just put # at the beginning of the expression. In GIVEN section, return type of the expression has to be the same one of type of the column. In EXPECT section the type could also just a boolean with the result of the assertion and you can use actualValue identifier to access value to check.
ruleCheatSheetBackground=If the same GIVEN data is shared by multiple test scenarios, you can use the Background tab to define them once. You can create a column in Background in the same way of Model
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it end with '.'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -189,3 +189,6 @@ validationSucceed=Validation Succeed

backgroundTabTitle=Background
backgroundErrorNotification=There are errors in Background data, please have a look
ruleCheatSheet23=To specify a MVEL expression (or invoke a Java method) just put # at the beginning of the expression. In GIVEN section, return type of the expression has to be the same one of type of the column. In EXPECT section the type could also just a boolean with the result of the assertion and you can use actualValue identifier to access value to check.
ruleCheatSheetBackground=If the same GIVEN data is shared by multiple test scenarios, you can use the Background tab to define them once. You can create a column in Background in the same way of Model
dmnCheatSheetBackground=If the same GIVEN data is shared by multiple test scenarios, you can use the Background tab to define them once. You can create a column in Background in the same way of Model.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dmnCheatSheetBackground and ruleCheatSheetBackground are the same cannot we have only one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielezonca I agree with @dupliaka, I suggest to define a separate method (eg. setCommonCheatSheetContent()) where shared descriptions are set in one single point and with a single property definition, in order to avoid text duplication.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the duplication (commonCheatSheetBackground) but I prefer not to refactor the whole method to create a setCommonCheatSheetContent because the structure of the HTML is different. To do that we should refactor the whole panel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also since we discussed it with you @danielezonca the dmn and drl bkg are not the same;
So drl can use it as a batch update but dmn can not and dmn would take only last record of it

@@ -189,3 +189,6 @@ validationSucceed=Validation Succeed

backgroundTabTitle=Background
backgroundErrorNotification=There are errors in Background data, please have a look
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that we shell mention all of the limitation for expressions?
Bcs we are obviosly do not mention that if you will /0 the expression will fall, why should we mention about the return type? Just my feeling of the cheat sheet idea. I would like to only mention a key words.
# specifies expression you can use to invoke Java code.
actualValue specifies parameter for assertion in EXPECT.
But if you have other ideas...

If you really want to leave it here a shorten version of it as well:
Return type of the expression in GIVEN section should be the same as type of the column.
Return type of the expression in EXPECT section should be boolean assertion result.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove the additional information but I think it could be useful to have them here. I cannot user your suggested version because in EXPECT expression can return the same type (and in this case it will be verified as == to the actualValue) or a boolean so I mentioned both

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, what about using /expression? Coz we are no need to put # before the expression for those column.

@yesamer
Copy link
Member

yesamer commented Jan 16, 2020

jenkins retest this please

@yesamer yesamer self-requested a review January 20, 2020 13:16
@kaldesai
Copy link

Hey @danielezonca,

Just some minor suggestions:

In rule-based:
Original: If the same GIVEN data is shared by multiple test scenarios, you can use the Background tab to define them once. The way to create a column in the Background section is the same as of the Model one.

Updated: If the same GIVEN data is shared with the multiple test scenarios, you can use the Background tab to define them only once. The way to create a column in the Background tab is the same as of the Model tab.
Bold the Background and Model.

Copy link
Member

@dupliaka dupliaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link

@kaldesai kaldesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the single comment, everything looks good to me. Thank you.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@danielezonca danielezonca merged commit a2ec0e8 into kiegroup:master Jan 22, 2020
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

Successfully merging this pull request may close these issues.

4 participants