Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable10] UI tests for delete feature #28811

Merged
merged 1 commit into from
Aug 28, 2017
Merged

[stable10] UI tests for delete feature #28811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 85 additions & 6 deletions tests/ui/features/bootstrap/FilesContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Behat\MinkExtension\Context\RawMinkContext;
use Behat\Gherkin\Node\TableNode;
use Page\FilesPage;
use SensioLabs\Behat\PageObjectExtension\PageObject\Exception\ElementNotFoundException;

require_once 'bootstrap.php';

Expand Down Expand Up @@ -135,8 +136,60 @@ public function iRenameTheFileToOneOfThisNames($fromName, TableNode $table)

}

/**
* @When I delete the file/folder :name
* @param string $name
* @return void
*/
public function iDeleteTheFile($name) {
$session = $this->getSession();
$this->filesPage->waitTillPageIsLoaded($session);
$this->filesPage->deleteFile($name, $session);
}

/**
* @When I delete the following file/folder
* @param TableNode $namePartsTable table of parts of the file name
* table headings: must be: |name-parts |
* @return void
*/
public function iDeleteTheFollowingFile(TableNode $namePartsTable) {
$fileNameParts = [];

foreach ($namePartsTable as $namePartsRow) {
$fileNameParts[] = $namePartsRow['name-parts'];
}
$this->filesPage->waitTillPageIsLoaded($this->getSession());
$this->filesPage->deleteFile($fileNameParts, $this->getSession());
}

/**
* @When I batch delete the marked files
* @return void
*/
public function iBatchDeleteTheMarkedFiles() {
$this->filesPage->deleteAllSelectedFiles($this->getSession());
}

/**
* @When I mark these files for batch action
* @param TableNode $files table of file names
* table headings: must be: |name|
* @return void
*/
public function iMarkTheseFilesForBatchAction(TableNode $files) {
$this->filesPage->waitTillPageIsLoaded($this->getSession());
foreach ($files as $file) {
$this->filesPage->selectFileForBatchAction(
$file['name'], $this->getSession()
);
}
}

/**
* @Then the file/folder :name should be listed
* @param string $name
* @return void
*/
public function theFileFolderShouldBeListed($name)
{
Expand All @@ -146,21 +199,47 @@ public function theFileFolderShouldBeListed($name)
}

/**
* @Then the following file/folder should be listed
* @Then the file/folder :name should not be listed
* @param string $name
* @return void
*/
public function theFileFolderShouldNotBeListed($name) {
$message = null;
try {
$this->filesPage->findFileRowByName($name, $this->getSession());
} catch (ElementNotFoundException $e) {
$message = $e->getMessage();
}
if (is_array($name)) {
$name = implode($name);
}
PHPUnit_Framework_Assert::assertEquals(
"could not find file with the name '" . $name . "'",
$message
);
}

/**
* @Then /^the following (?:file|folder) should (not|)\s?be listed$/
* @param string $shouldOrNot
* @param TableNode $namePartsTable table of parts of the file name
* table headings: must be: |name-parts |
*/
public function theFollowingFileFolderShouldBeListed(TableNode $namePartsTable)
{
public function theFollowingFileFolderShouldBeListed(
$shouldOrNot, TableNode $namePartsTable
) {
$should = ($shouldOrNot !== "not");
$fileNameParts = [];

foreach ($namePartsTable as $namePartsRow) {
$fileNameParts[] = $namePartsRow['name-parts'];
}

PHPUnit_Framework_Assert::assertNotNull(
$this->filesPage->findFileRowByName($fileNameParts, $this->getSession())
);
if ($should) {
$this->theFileFolderShouldBeListed($fileNameParts);
} else {
$this->theFileFolderShouldNotBeListed($fileNameParts);
}
}

/**
Expand Down
71 changes: 71 additions & 0 deletions tests/ui/features/files/delete.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
Feature: delete

Background:
Given a regular user exists
And I am logged in as a regular user
And I am on the files page

Scenario: Delete files & folders one by one and check its existence after page reload
When I delete the folder "simple-folder"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be only one when per scenario. many "Given" one "When" and many "Then"

Copy link
Contributor Author

@phil-davis phil-davis Aug 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true in theory. There should be some "Given" steps to set things up (they might all be in the Background: like here), then a set of "When" (with maybe a few steps) to carry out the actions, then a set of "Then" that check the result.

So we could easily make separate scenarios here.

The practical side for UI tests, is that the "test setup" phase - creating user, logging in, navigating to the desired page... - takes so long. So to speed it up, it is practical to make multiple when-then in a scenario.

Also, sometimes there are genuinely mixed sequences of when-then needed. e.g. even if only 1 file delete is tested here, we want to:

  1. when "delete the file"
  2. then "see if it is gone from the local page"
  3. when "reload the page"
  4. then "see if it really is gone"
    because if the Javascript/AJAX is not working properly, (2) and/or (4) might fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a sample discussion of this issue, where there is a "workflow" to be acceptance tested:
https://stackoverflow.com/questions/12060011/is-it-acceptable-to-write-a-given-when-then-when-then-test-in-gherkin
and of course there are different opinions on how best to do it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this approach, it is like a test script. You could use given-and as preconditions, or even hiding those thens after the whens by checking it directly in the code.

There should be only one thing marked as when, rest are preconditions or postconditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The step sequence could be re-ordered so the "when" all come first:

	Scenario: Delete files & folders one by one and check its existence after page reload
		When I delete the folder "simple-folder"
		And I delete the file "lorem.txt"
		And I delete the folder "strängé नेपाली folder"
		And I delete the file "strängé filename (duplicate #2).txt"
		Then the folder "simple-folder" should not be listed
		And the file "lorem.txt" should not be listed
		And the folder "strängé नेपाली folder" should not be listed
		And the file "strängé filename (duplicate #2).txt" should not be listed
		When the files page is reloaded
		Then the folder "simple-folder" should not be listed
		And the folder "strängé नेपाली folder" should not be listed
		And the file "lorem.txt" should not be listed
		And the file "strängé filename (duplicate #2).txt" should not be listed

the last "When the files page is reloaded" could be renamed to "And the files page is reloaded" - that just "hides" the fact that it is an action, and makes no real difference.

Maybe it could be cut down to:

	Scenario: Delete files & folders one by one and check its existence after page reload
		When I delete the folder "simple-folder"
		And I delete the file "lorem.txt"
		And I delete the folder "strängé नेपाली folder"
		And I delete the file "strängé filename (duplicate #2).txt"
		Then the deleted items should not be listed

and the code can remember which files have been deleted, and code for Then the deleted items should not be listed can check for the files on the current page, then reload the page and check again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, this is a back-port of a commit in master. Whatever we change needs to be changed in master and also back-ported.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it could be a table

            When I delete the elements
              | simple-folder |
              | lorem.txt |
                 (...)
	Then the deleted items should not be listed

If the difference between folder and element is required, using another column detailing folder/file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SergioBertolinSG Let me merge this as its only a backport and I will do a new PR against master

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Then the folder "simple-folder" should not be listed
When I delete the file "lorem.txt"
Then the file "lorem.txt" should not be listed
When I delete the folder "strängé नेपाली folder"
Then the folder "strängé नेपाली folder" should not be listed
When I delete the file "strängé filename (duplicate #2).txt"
Then the file "strängé filename (duplicate #2).txt" should not be listed
When the files page is reloaded
Then the folder "simple-folder" should not be listed
And the folder "strängé नेपाली folder" should not be listed
And the file "lorem.txt" should not be listed
And the file "strängé filename (duplicate #2).txt" should not be listed

Scenario: Delete a file with problematic characters
When I rename the following file to
|from-name-parts |to-name-parts |
|lorem.txt |'single' |
| |"double" quotes|
| |question? |
| |&and#hash |
And the files page is reloaded
Then the following file should be listed
|name-parts |
|'single' |
|"double" quotes|
|question? |
|&and#hash |
And I delete the following file
|name-parts |
|'single' |
|"double" quotes|
|question? |
|&and#hash |
Then the following file should not be listed
|name-parts |
|'single' |
|"double" quotes|
|question? |
|&and#hash |
And the files page is reloaded
Then the following file should not be listed
|name-parts |
|'single' |
|"double" quotes|
|question? |
|&and#hash |

Scenario: Delete multiple files at once
When I mark these files for batch action
|name |
|data.zip|
|lorem.txt|
|simple-folder|
And I batch delete the marked files
Then the folder "simple-folder" should not be listed
And the file "data.zip" should not be listed
And the file "lorem.txt" should not be listed
When the files page is reloaded
Then the folder "simple-folder" should not be listed
And the file "data.zip" should not be listed
And the file "lorem.txt" should not be listed

59 changes: 55 additions & 4 deletions tests/ui/features/lib/FilesPage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class FilesPage extends OwnCloudPage
//TODO make simpler, only ID .//*[@id='fileList']
protected $fileListXpath = ".//div[@id='app-content-files']//tbody[@id='fileList']";
protected $loadingIndicatorXpath = ".//*[@class='loading']";
protected $deleteAllSelectedBtnXpath = ".//*[@id='app-content-files']//*[@class='delete-selected']";

private $strForNormalFileName = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890';

Expand Down Expand Up @@ -136,6 +137,7 @@ public function findFileRowByName($name, Session $session)
{
$previousFileCount = 0;
$currentFileCount = null;
$this->scrollToPosition('#' . $this->appContentId, 0, $session);

if (is_array($name)) {
// Concatenating separate parts of the file name allows
Expand Down Expand Up @@ -219,10 +221,11 @@ public function closeSharingDialog() {
*/
public function scrollDownAppContent (Session $session)
{
$session->evaluateScript(
'$("#' . $this->appContentId . '").scrollTop($("#' . $this->appContentId . '")[0].scrollHeight);'
$this->scrollToPosition(
'#' . $this->appContentId,
'$("#' . $this->appContentId . '")[0].scrollHeight',
$session
);

$this->waitForOutstandingAjaxCalls($session);
}

Expand Down Expand Up @@ -277,6 +280,55 @@ public function renameFile($fromFileName, $toFileName, Session $session, $maxRet
}
}

/**
*
* @param string $name
* @param Session $session
* @return void
*/
public function deleteFile($name, Session $session) {
$row = $this->findFileRowByName($name, $session);
$row->delete();
$this->waitForAjaxCallsToStartAndFinish($session);
}

/**
*
* @throws ElementNotFoundException
* @return \Behat\Mink\Element\NodeElement
*/
public function findDeleteAllSelectedFilesBtn() {
$deleteAllSelectedBtn = $this->find(
"xpath", $this->deleteAllSelectedBtnXpath
);
if (is_null($deleteAllSelectedBtn)) {
throw new ElementNotFoundException(
"could not find button to delete all selected files"
);
}
return $deleteAllSelectedBtn;
}

/**
*
* @param Session $session
* @return void
*/
public function deleteAllSelectedFiles(Session $session) {
$this->findDeleteAllSelectedFilesBtn()->click();
$this->waitForAjaxCallsToStartAndFinish($session);
}

/**
*
* @param string $name
* @param Session $session
* @return void
*/
public function selectFileForBatchAction($name, Session $session) {
$row = $this->findFileRowByName($name, $session);
$row->selectForBatchAction();
}
/**
*
* @param int $number
Expand All @@ -298,7 +350,6 @@ public function findFileActionsMenuBtnByNo($number) {
* @return void
*/
public function clickFileActionsMenuBtnByNo($number) {

$this->findFileActionsMenuBtnByNo($number)->click();
}

Expand Down
3 changes: 2 additions & 1 deletion tests/ui/features/lib/FilesPageElement/FileActionsMenu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ public function rename(
* @return void
*/
public function delete() {
;
$deleteBtn = $this->findButton($this->deleteActionLabel);
$deleteBtn->click();
}

/**
Expand Down
31 changes: 26 additions & 5 deletions tests/ui/features/lib/FilesPageElement/FileRow.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use Page\OwncloudPage;
use SensioLabs\Behat\PageObjectExtension\PageObject\Exception\ElementNotFoundException;
use Behat\Mink\Element\NodeElement;
use Behat\Mink\Session;

/**
* Object of a row on the FilesPage
Expand Down Expand Up @@ -167,14 +168,24 @@ public function findRenameInputField() {
*/
public function rename($toName) {
$actionMenu = $this->openFileActionsMenu();
$actionMenu->rename($this->fileRenameInputXpath);
$actionMenu->rename();
$this->waitTillElementIsNotNull($this->fileRenameInputXpath);
$inputField = $this->findRenameInputField();
$this->cleanInputAndSetValue($inputField, $toName);
$inputField->blur();
$this->waitTillElementIsNull($this->fileBusyIndicatorXpath);
}

/**
* deletes the file
*
* @return void
*/
public function delete() {
$actionMenu = $this->openFileActionsMenu();
$actionMenu->delete();
}

/**
* finds and returns the tooltip element
*
Expand All @@ -191,6 +202,15 @@ public function findTooltipElement() {
return $element;
}

/**
* returns the tooltip text
*
* @return string
*/
public function getTooltip() {
return $this->findTooltipElement()->getText();
}

/**
* finds and returns the thumbnail of the file
*
Expand All @@ -206,12 +226,13 @@ public function findThumbnail() {
}
return $thumbnail;
}

/**
* returns the tooltip text
* selects this row for batch action e.g. download or delete
*
* @return string
* @return void
*/
public function getTooltip() {
return $this->findTooltipElement()->getText();
public function selectForBatchAction() {
$this->findThumbnail()->click();
}
}
14 changes: 14 additions & 0 deletions tests/ui/features/lib/OwncloudPage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,20 @@ public function getWindowHeight($session)
);
}

/**
* scrolls to a position in a specified element
*
* @param string $jQuerySelector e.g. "#app-content"
* @param int|string $position number or JS function that returns a number
* @param Session $session
* @return void
*/
public function scrollToPosition($jQuerySelector, $position, Session $session) {
$session->evaluateScript(
'$("' . $jQuerySelector . '").scrollTop(' . $position . ');'
);
}

/**
* waits till all ajax calls are finished (jQuery.active === 0)
* @param Session $session
Expand Down