On Fri, Apr 18, 2008 at 7:54 PM, Vincent Massol <[email protected]> wrote:

I think we should also provides the French translation by default since lots of us are French and can translate it easily...

True; done.

Also it might be good to follow the naming conventions started by Sergiu.

I thought copy/rights/actions were generic enough to be included like this.

...

+ 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();

Indeed; done.

...

+ /** + * 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?

The setSpeed() affects the assert that follows (without this setSpeed the test fails).

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.

My bad, all the tests were grouped in Evelina's version, I've re-grouped them. Thanks, -- Jean-Vincent Drean