David Monllaó added a comment - 25/Oct/13 9:53 AM Hi Rossie, It looks good, just a few comments: You can switch steps like https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R64 to Then I should see "Comments" in the "Comments" "block" Would be good to restrict some checkings to a smaller scope when using strings as common as comments https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R55 you can use I should see "Comments" in the "Comments" "block" for example if the comments block should appear or you can use ... in the "region-pre" "region" if not Please follow the correct indentation, see other .feature files examples The usage of Given/When/Then is not correct, you can only pass through them in this direction Given -> When -> Then Any particular reason to leave that much space between steps Seeing how long the test is and that you are merging two MDLQAs probably would make sense to split the test in 2 different scenarios according to the MDLQAs descriptions, make sure that the scenario descriptions are explaining what is the scenario testing

David Monllaó added a comment - 25/Oct/13 9:54 AM To split the scenario in two you might be interested in using the background section for the shared parts http://docs.behat.org/guides/1.gherkin.html#backgrounds

Rossiani Wijaya added a comment - 28/Oct/13 9:27 AM Hi David, I updated the patch according to your suggestion. However, I'm not sure if I used "when > then" correctly. When you have a chance, could you please review this again? Thanks

David Monllaó added a comment - 28/Oct/13 9:33 AM Hi Rossie, have you pushed your changes? I still see most (if not all) the same issues I commented about

Rossiani Wijaya added a comment - 28/Oct/13 9:46 AM Sorry, I just pushed the changes.

David Monllaó added a comment - 29/Oct/13 2:39 AM Looks very good Rossie, thanks, only 2 minor issues: For what I see you can expand the Background section until https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R50 as those steps are repeated in both scenarios The Given/When/Then prefixes separates the scenarios in 3 parts we should not return to When after a Then, the prefixes only goes in one direction (I've just updated the MDocs page http://docs.moodle.org/dev/Acceptance_testing#Writing_features point #5.4 to give more info about it) From https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R53 for example, all should be And, same in the other scenario https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R76

Rossiani Wijaya added a comment - 29/Oct/13 5:19 AM Thanks David. Updated the patch and sending for integration review.

Dan Poltawski added a comment - 01/Nov/13 9:46 AM Hi Rosie, I think this should be back ported to 2.5?

Rossiani Wijaya added a comment - 04/Nov/13 2:18 AM Yes, it should. I backported the patch to 2.5 and tested. Behat test passed for 2.5.

Dan Poltawski added a comment - 04/Nov/13 4:01 PM Hi Rosie, Looks good on 25, but running on master at the moment i'm getting fails: .................................F.................................... 70 .......................................F.............................. 140 ............................   (::) failed steps (::)   01. Xpath matching locator "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" not found. In step `And I should see "Comments" in the "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" "xpath_element"'. # behat_general::assert_element_contains_text() From scenario `Add and configure a block throughtout the site'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/configure_block_throughout_site.feature:8 Of feature `Add and configure blocks throughout the site'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/configure_block_throughout_site.feature   02. Css matching locator "#region-post" not found. In step `And I should see "Comments" in the "#region-post" "css_element"'. # behat_general::assert_element_contains_text() From scenario `Block settings can be modified so that a block can be hidden or moved'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/manage_blocks.feature:64 Of feature `Block appearances'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/manage_blocks.feature   8 scenarios (6 passed, 2 failed) 168 steps (166 passed, 2 failed) 5m16.962s

Dan Poltawski added a comment - 05/Nov/13 3:07 AM My bad, this was caused by me running in clean.

Dan Poltawski added a comment - 05/Nov/13 3:40 AM Integrated to msater and 25 - thanks Rosie

Ankit Agarwal added a comment - 05/Nov/13 10:07 AM works as described. thanks

Damyon Wiese added a comment - 08/Nov/13 7:52 AM Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

Eloy Lafuente (stronk7) added a comment - 23/Apr/14 6:02 AM FYI: related MDLQA-1545 and MDLQA-1546 have been moved from MDLQA-1 to MDLQA-5249 (bag of behat-converted tests). Thanks!