12 Nov
2012
12 Nov
'12
4:54 p.m.
On Nov 12, 2012, at 4:32 PM, Caleb James DeLisle <calebdelisle(a)lavabit.com> wrote:
Hi,
These guidelines are ok assuming the tests are only run once before clearing the database
as the
CI server does. There are a couple of tests which generate randomly named documents and
if you run
the test suite multiple times without clearing the database, eventually they build up and
cause
a registration test to fail.
This is because the registration test is bad and should be fixed.
I think it's wrong to leave randomly named garbage
in the wiki because
it has an inherent collision risk and increases the overall entropy of the tests which we
want to avoid.
The rule is that each test should create pages with the space = test class and page = test
name. You can then use a prefix if you need more than 1.
Pages shouldn't be random.
<off topic for this thread>
What I most disagree about is that this test is
inherently broken because it doesn't use page objects,
it's about 10 lines of code and it's worked up until now. IMO page objects are a
tool, if you don't like
something because it doesn't use that tool, that's a personal preference.
This test is very bad and doesn't follow any rule that we've set up…
* It uses cryptic xpath in the test itself and those should be put only in PageObjects
* It's hard to read and maintain and doesn't have a comment to explain what it
does
* It's complex and reuses the same code for different runs, basically it creates a
test framework in the test itself which should be avoided
FTR here's snippet from the Test class:
@Override
protected boolean tryToRegister()
{
registrationPage.clickRegister();
registrationPage.waitUntilElementsAreVisible(
new By[]
{By.xpath("//td[@class='username']/a[@href='/xwiki/bin/view/XWiki/JohnSmith']"),
By.xpath("//dd/span[@class='LV_validation_message
LV_invalid']")
},
false
);
return !getDriver()
.findElements(
By.xpath("//td[@class='username']/a[@href='/xwiki/bin/view/XWiki/JohnSmith']"))
.isEmpty();
}
As I've pointed out on IRC the good strategy for this is to use an existing Page
Object. Here's how it should be written instead IMO:
UsersAdministrationSectionPage page = (UsersAdministrationSectionPage)
registrationPage.clickRegister();
LiveTableElement usersTable = page.getUsersTable();
usersTable.filterColumn("Username", "JohnSmith");
return usersTable.getRowCount() == 1;
But I'd also not create a framework here since that makes the test hard to
read/support.
</off topic for this thread>
So -0 to tests naming documents using the random
number generator.
Yes, random pages should be banned.
Thanks
-Vincent
Thanks,
Caleb
On 11/12/2012 04:02 AM, Vincent Massol wrote:
>
> On Nov 12, 2012, at 9:56 AM, Sorin Burjan <sorin.burjan(a)xwiki.com> wrote:
>
>> Hello Vincent,
>>
>> I agree with your proposals. I was thinking this is already a rule,
>> especially 1) and 2)
>
> yes me too but since I was having a discussion with Caleb on IRC this morning and he
didn't fully agree about it, I'm resending it as an official vote (I don't
think it ever made it as a proper vote, more something we were doing
"unofficially").
>
> Thanks
> -Vincent
>
>> I agree with them.
>>
>> Regards,
>> Sorin B.
>>
>>
>> On Mon, Nov 12, 2012 at 10:50 AM, Vincent Massol <vincent(a)massol.net>
wrote:
>>
>>> Hi devs,
>>>
>>> I've just added 3 items on
>>>
http://dev.xwiki.org/xwiki/bin/view/Community/Testing#HSelenium2-basedFrame… I'd
like to ensure that we agree about them (if not I'll remove them):
>>>
>>> * Tests must not depend on one another. In other words, it should be
>>> possible to execute tests in any order and running only one test class
>>> should work fine.
>>> * Test chat need to change existing configuration (e.g. change the default
>>> language, set specific rights, etc) must put back the configuration as it
>>> was
>>> * Tests are allowed to create new documents and don't need to remove
them
>>> at the end of the test
>>>
>>> Here's my +1
>>>
>>> Thanks
>>> -Vincent