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- - xwiki-devs@xwiki.org

18 Apr 2008

On Fri, Apr 18, 2008 at 7:54 PM, Vincent Massol &lt;vincent(a)massol.net&gt; 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