Re: [xwiki-devs] [xwiki-notifications] r9235 - xwiki-platform/core/trunk/xwiki-core/src/main/resources xwiki-platform/skins/trunk/albatross/src/main/resources/albatross xwiki-platform/skins/trunk/toucan/src/main/resources/toucan xwiki-products/xwiki-enterprise/trunk/distribution-test/selenium-tests/src/test/it/com/xpn/xwiki/it/selenium xwiki-products/xwiki-enterprise/trunk/wiki/src/main/resources/Main xwiki-products/xwiki-enterprise/trunk/wiki/src/main/resources/XWiki
Some quick comments below. First let me say: Very cool tests! This is really cool and needed and it's good to see we're improving our tests. Well done Evelina/JV. On Apr 18, 2008, at 4:14 PM, jvdrean (SVN) wrote:
Author: jvdrean Date: 2008-04-18 16:14:39 +0200 (Fri, 18 Apr 2008) New Revision: 9235
Added: xwiki-products/xwiki-enterprise/trunk/distribution-test/selenium- tests/src/test/it/com/xpn/xwiki/it/selenium/AllDocsTest.java xwiki-products/xwiki-enterprise/trunk/wiki/src/main/resources/XWiki/ Tableresults xwiki-products/xwiki-enterprise/trunk/wiki/src/main/resources/XWiki/ Tableview Modified: xwiki-platform/core/trunk/xwiki-core/src/main/resources/ ApplicationResources.properties xwiki-platform/skins/trunk/albatross/src/main/resources/albatross/ usersandgroups.css xwiki-platform/skins/trunk/toucan/src/main/resources/toucan/ usersandgroups.css xwiki-products/xwiki-enterprise/trunk/distribution-test/selenium- tests/src/test/it/com/xpn/xwiki/it/selenium/AllTests.java xwiki-products/xwiki-enterprise/trunk/wiki/src/main/resources/Main/ AllDocs Log: XE-221 : Change the table that displays all the documents (Main.AllDocs) to use ajax for more rapid loading XE-224 : Write selenium test to verify the behaviour of the AllDocs ajax table
Applied patch from Evelina with few modifications.
Modified: xwiki-platform/core/trunk/xwiki-core/src/main/resources/ ApplicationResources.properties
I think we should also provides the French translation by default since lots of us are French and can translate it easily... Also it might be good to follow the naming conventions started by Sergiu. [snip]
Modified: xwiki-platform/skins/trunk/albatross/src/main/resources/ albatross/usersandgroups.css =================================================================== --- xwiki-platform/skins/trunk/albatross/src/main/resources/ albatross/usersandgroups.css 2008-04-18 10:34:43 UTC (rev 9234) +++ xwiki-platform/skins/trunk/albatross/src/main/resources/ albatross/usersandgroups.css 2008-04-18 14:14:39 UTC (rev 9235) @@ -1,7 +1,6 @@ /* USERS SCROLLING SYSTEM */ - div.scrollbar { - overflow: auto; + overflow: auto !important;
Is that going to change the rights and users tables? [snip]
-#userstable, #groupstable, #usersandgroupstable { +#userstable, #groupstable, #usersandgroupstable, #alldocstable {
I think we should rename the file usersandgroups.css since it's no longer about users and groups! :) (same for Toucan) [snip]
Added: xwiki-products/xwiki-enterprise/trunk/distribution-test/ selenium-tests/src/test/it/com/xpn/xwiki/it/selenium/AllDocsTest.java
[snip]
+ public void setUp() throws Exception + { + super.setUp(); + loginAsAdmin(); + open(getUrl("Main", "AllDocs")); + } + + /** + * Validate presence of "Actions" column in table view for administrator. Validate absence of + * "Actions" column for users without administration rights. + */ + public void testTableViewActionsPresence() + { + assertElementPresent("//td[text()='Actions']"); + + // Verify that the user is logged in and log out + if (isAuthenticated()) { + logout();
Why is that needed since the setup always does a loginAsAdmin? I think this can be changed to: logout();
+ } + assertElementNotPresent("//td[text()='Actions']"); + } + + /** + * Validate input suggest for Page field. + */ + public void testTableViewSuggestForPage() + { + getSelenium().typeKeys("page", "Treeview"); + // The table is updated via Ajax, we give it the time to make this call + getSelenium().setSpeed("1000"); + assertElementPresent("//td[@class='pagename']/ a[text()='Treeview']"); + getSelenium().setSpeed("0");
Is the setSpeed() required? We don't do any action after it's set so is it used for anything? Shouldn't it be before the typeKeys? Also if it's required I think we should introduce a method in our DSL for this that does setspeed, type keys and reset speed. [snip] There are way too many test methods. These are NOT unit tests and thus 1) are costly and 2) are not supposed to be independent of each other. Thus most of them should be grouped in the same test method IMO, with nice comments separating them. Maybe we don't need to have a single test method but they should be grouped according to what they are doing. [snip] Thanks -Vincent
Vincent Massol wrote:
Is the setSpeed() required? We don't do any action after it's set so is it used for anything? Shouldn't it be before the typeKeys?
Also if it's required I think we should introduce a method in our DSL for this that does setspeed, type keys and reset speed.
I think that I created a bad precedent. The idea is that I needed to write some tests quickly for something involving asynchronous requests, and I was lazy enough not to search the proper way to do this in Selenium, so I just delayed the commands. I knew it involved waitForCondition, but didn't know what the condition was. In the meantime I found this: http://thoughthaus.net/blog/?page_id=116 -- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (2)
-
Sergiu Dumitriu -
Vincent Massol