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.

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

Add tests to cover bottom button functionality. #4222

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
namespace VuFindTest\Mink;

use Behat\Mink\Element\Element;
use Behat\Mink\Session;

/**
* Mink bulk action test class.
Expand Down Expand Up @@ -279,27 +280,70 @@ public function testBulkDeleteFromList(): void
$this->unfindCss($page, 'button[name="delete"]');
}

/**
* Data provider to allow testing of top or bottom controls.
*
* @return array[]
*/
public static function topOrBottomProvider(): array
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also be used for all of the other buttons?

Copy link
Member Author

@demiankatz demiankatz Feb 13, 2025

Choose a reason for hiding this comment

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

Ideally, yes, but many of the other tests make changes to the data in the system (like deleting items from lists), so they are not repeatable in their current form. We would have to do some work to reset the state after each iteration, and I didn't have time to invest in figuring out all the details. I figured some testing was better than none. If you think we should do that additional work before merging this, we can add a TODO checkbox and I can work on it as time permits.

{
return [
'top button' => [''],
'bottom button' => ['bottom_'],
];
}

/**
* After a failed button click has been detected, resize the window and try again.
*
* @param Session $session Mink session
* @param Element $page Current page element
* @param string $selector Selector to click
*
* @return void
*/
protected function retryClickWithResizedWindow(Session $session, Element $page, string $selector): void
{
// For some reason, the click action does not always succeed here; resizing
// the window and retrying seems to prevent intermittent test failures.
echo "\n\nMink click failed; retrying with resized window!\n";
$session->resizeWindow(1280, 200, 'current');
$this->clickCss($page, $selector);
$session->resizeWindow(1280, 768, 'current');
}

/**
* Test that the export control works.
*
* @param string $idPrefix Prefix for bulk control IDs.
*
* @return void
*
* @dataProvider topOrBottomProvider
*/
public function testBulkExport(): void
public function testBulkExport(string $idPrefix): void
{
$session = $this->getMinkSession();
$page = $this->setUpGenericBulkTest();
$button = $this->findCss($page, '#ribbon-export');
$buttonSelector = '#' . $idPrefix . 'ribbon-export';

// First try clicking without selecting anything:
$button->click();
$this->clickCss($page, $buttonSelector);
$this->checkForNonSelectedMessage($page);
$this->closeLightbox($page, true);

// Now do it for real -- we should get a lightbox prompt.
$page->find('css', '#addFormCheckboxSelectAll')->check();
$button->click();
$page->find('css', '#' . $idPrefix . 'addFormCheckboxSelectAll')->check();
$this->waitStatement('$("input.checkbox-select-item:checked").length === 2');
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this waitStatement to both of the tests so that they match other earlier tests with the same behavior. It was an early attempt to fix the click-non-registration problem. It didn't help. I'm not sure if we should leave this in or take it out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this statement makes sense, because the item checkboxes are selected with js and we have no guarantee that they are selected when button is clicked without it.

$this->clickCss($page, $buttonSelector);

// Select EndNote option
$select = $this->findCss($page, '#format');
try {
$select = $this->findCss($page, '#format', 100);
} catch (\Exception $e) {
$this->retryClickWithResizedWindow($session, $page, $buttonSelector);
$select = $this->findCss($page, '#format');
}
$select->selectOption('EndNote');

// Do the export:
Expand All @@ -312,23 +356,32 @@ public function testBulkExport(): void
/**
* Test that the print control works.
*
* @param string $idPrefix Prefix for bulk control IDs.
*
* @return void
*
* @dataProvider topOrBottomProvider
*/
public function testBulkPrint(): void
public function testBulkPrint(string $idPrefix): void
{
$session = $this->getMinkSession();
$page = $this->setUpGenericBulkTest();
$button = $this->findCss($page, '#ribbon-print');
$buttonSelector = '#' . $idPrefix . 'ribbon-print';

// First try clicking without selecting anything:
$button->click();
$this->clickCss($page, $buttonSelector);
$this->checkForNonSelectedMessage($page);
$page->find('css', '.modal-body .btn')->click();

// Now do it for real -- we should get redirected.
$page->find('css', '#addFormCheckboxSelectAll')->check();
$button->click();
$page->find('css', '#' . $idPrefix . 'addFormCheckboxSelectAll')->check();
$this->waitStatement('$("input.checkbox-select-item:checked").length === 2');
$this->clickCss($page, $buttonSelector);
[, $params] = explode('?', $session->getCurrentUrl());
if (str_starts_with($params, 'lookfor')) {
$this->retryClickWithResizedWindow($session, $page, $buttonSelector);
[, $params] = explode('?', $session->getCurrentUrl());
}
$this->assertEquals(
'print=true&id[]=Solr|testsample1&id[]=Solr|testsample2',
str_replace(['%5B', '%5D', '%7C'], ['[', ']', '|'], $params)
Expand Down
Loading