Last modified: 2014-10-26 17:21:15 UTC
The only sleep that we have in tests is here: https://github.com/wikimedia/qa-browsertests/blob/master/features/step_definitions/upload_wizard_steps.rb#L17 The code should be refactored so the sleep is removed.
We are using "sleep" in several places. Sleep is evil. We should remove it from all places.
There are more robust ways to wait for something to happen than using sleep: http://watirwebdriver.com/waiting/ https://github.com/cheezy/page-object/wiki/Ajax-Calls
Sleep statements found: browsertests https://github.com/wikimedia/qa-browsertests/blob/master/features/step_definitions/print_export_menu_steps.rb#L14 https://github.com/wikimedia/qa-browsertests/blob/master/features/step_definitions/print_export_menu_steps.rb#L23 https://github.com/wikimedia/qa-browsertests/blob/master/features/step_definitions/upload_wizard_steps.rb#L23 https://github.com/wikimedia/qa-browsertests/blob/master/features/step_definitions/upload_wizard_steps.rb#L77 CirrusSearch https://github.com/wikimedia/mediawiki-extensions-CirrusSearch/blob/master/tests/browser/features/step_definitions/search_steps.rb#L82 TwnMainPage https://github.com/wikimedia/mediawiki-extensions-TwnMainPage/blob/master/tests/browser/features/step_definitions/signed_in_and_approved_user_steps.rb#L66 VisualEditor https://github.com/wikimedia/mediawiki-extensions-VisualEditor/blob/master/modules/ve-mw/test/browser/features/step_definitions/bullets_steps.rb#L17 https://github.com/wikimedia/mediawiki-extensions-VisualEditor/blob/master/modules/ve-mw/test/browser/features/step_definitions/shared_steps.rb#L25 https://github.com/wikimedia/mediawiki-extensions-VisualEditor/blob/master/modules/ve-mw/test/browser/features/step_definitions/transclusion_steps.rb#L31 Wikibase https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/selenium_cuc/features/step_definitions/aliases_steps.rb#L56 https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/selenium_cuc/features/support/modules/entity_module.rb#L115 https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/selenium_cuc/features/support/modules/entity_module.rb#L117 https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/selenium_cuc/features/support/modules/reference_module.rb#L50
If you need more information (and you probably do), feel free to ask questions here, at #wikimedia-qa freenode IRC channel or at QA mailing list: https://lists.wikimedia.org/mailman/listinfo/qa
(In reply to comment #3) > Sleep statements found: > Removing the code from the following places will do ?
Tony Thomas: just removing the code is not enough. The tests have to be made to wait properly for whatever it is they are waiting for. Using sleep() is either an interim step that never got completed, or else the person writing the test could not think of a better way to wait for whatever the test needs. So the tests need to be updated in a thoughtful way so that they wait correctly without using sleep() but also without failing mistakenly.
Tony, if you need help getting started apply for Pair programming Friday for fun and profit: https://www.mediawiki.org/wiki/Pair_programming_Friday_for_fun_and_profit
Tony and I have paired on getting his environment set up to work on this. Tony, if you still are interested in working on this, feel free to assign the bug to yourself. Let me know if you need help.
(In reply to comment #8) > Tony and I have paired on getting his environment set up to work on this. I will put my status in [QA] once I have made my environment ready.
Change 108916 had a related patch set uploaded by Zfilipin: Replaced sleep with page-object gem waiting API https://gerrit.wikimedia.org/r/108916
Change 108918 had a related patch set uploaded by 01tonythomas: Removed sleep with waiting API in browsertests https://gerrit.wikimedia.org/r/108918
Change 108918 merged by jenkins-bot: Removed sleep with waiting API in browsertests https://gerrit.wikimedia.org/r/108918
Change 108916 merged by Cmcmahon: Replaced sleep with page-object gem waiting API https://gerrit.wikimedia.org/r/108916
Change 109471 had a related patch set uploaded by 01tonythomas: Removed sleep variable from TwnMainPage tests https://gerrit.wikimedia.org/r/109471
Cirrus has two sleeps: 1. It waits for some suggestions not to appear. Without reworking the error handling in the javascript I can't remove this. I don't feel up to reworking that right now. 2. It sleeps between the creation of two pages that it uses to test searches that prefer pages with recent edits. If I could backdate edit times then I wouldn't need it but I can't at this point. In total Cirrus sleeps 35 seconds because each of the above is executed once. It could be better but it could be worse.
Change 109471 merged by jenkins-bot: Removed sleep variable from TwnMainPage tests https://gerrit.wikimedia.org/r/109471
All patches merged => resetting status from PATCH_TO_REVIEW to NEW
Are there more tests containing sleep or is this fixed? Tony: Are you still working on this or should the assignee be reset to default?
(In reply to Andre Klapper from comment #18) > Are there more tests containing sleep or is this fixed? > > Tony: Are you still working on this or should the assignee be reset to > default? I think there are a few more sleep keywords to be removed. Sorry, but I dont think I will be able to continue on this right now. Resetting to default assignee for the time being.
We can keep this open. I actually just added a sleep to a test because I needed to get on with other things, and I should return to fix it at some point.
