Skip to content

Comments

Grouped keywords together#2

Open
sungysang wants to merge 1 commit intossong/learn-robot-frameworkfrom
ssong/improve-robot-test
Open

Grouped keywords together#2
sungysang wants to merge 1 commit intossong/learn-robot-frameworkfrom
ssong/improve-robot-test

Conversation

@sungysang
Copy link
Owner

No description provided.

Copy link

@Julian88Tex Julian88Tex left a comment

Choose a reason for hiding this comment

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

@sungysang provided some additional comments focused around using existing NPSP.robot keywords or at least using them as models and/or adding to the library.


*** Test Cases ***
Assign GAU to Campaign and Verify Allocation on Opportunity
${gau_id}= Create GAU

Choose a reason for hiding this comment

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

@sungysang looks like this can be part of Setup Test Data since it isn't part of the test.

Comment on lines +86 to +88
API Create Campaign GAU Allocation ${gau_id}
... &{campaign}[Id]
... ${ns}Percent__c=100.0

Choose a reason for hiding this comment

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

You could group this into a keyword called Create 100% Campaign GAU Allocation

Choose a reason for hiding this comment

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

If you want to leave this keyword on this robot file then I'd just hard code the API Create keyword to be 100%. If you want the keyword to be more generic and usable in other tests, then I would promote your API Create Keyword to NPSP.robot file (first I'd check to make sure one doesn't already exist that you can just use an update). And then you can create a localized keyword that references the above. While not a requirement, I recommend building keywords so that there are no variables in the tests so that the tests can be very readable steps.

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

If you promote it, you can probably put it near this keyword: https://github.com/SalesforceFoundation/NPSP/blob/master/robot/Cumulus/resources/NPSP.robot#L139

Copy link
Owner Author

Choose a reason for hiding this comment

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

@Julian88Tex Thanks, I basically copy pasted the original keyword since that one only worked for Opportunities, how would I promote it?

Choose a reason for hiding this comment

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

@sungysang promote it means cut it from this file and paste it in the NPSP.robot file

Comment on lines +89 to +90
Create Opportunity With Campaign ${data}[contact][LastName] Household
... ${campaign}[Name]

Choose a reason for hiding this comment

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

Any reason this needs to be UI? If you just testing the trigger logic, you might as well just create Opportunity through API. Looks like you can use this existing Keyword: https://github.com/SalesforceFoundation/NPSP/blob/master/robot/Cumulus/resources/NPSP.robot#L50

Copy link
Owner Author

Choose a reason for hiding this comment

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

@Julian88Tex I felt like this should try to re-create the user scenario as close as possible through the UI, but would you recommend API?

Choose a reason for hiding this comment

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

@sungysang it all depends on what you're trying to test. If you trying to test the trigger, then API should be fine IMO since I wouldn't expect UI to have any impact on trigger logic vs. making an API call. If you're trying to test UI then stick with UI.

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.

2 participants