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

Add tests to cover bottom button functionality. #4222

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Add tests to cover bottom button functionality. #4222

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5570190
Add tests to cover bottom button functionality.
demiankatz Jan 30, 2025
c97015e
Merge branch 'dev' into bottom-button-tests
demiankatz Jan 31, 2025
1aa0892
Different hack.
demiankatz Jan 31, 2025
2129fc5
Merge branch 'dev' into bottom-button-tests
demiankatz Feb 13, 2025
275856f
Review feedback.
demiankatz Feb 13, 2025
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
62 changes: 51 additions & 11 deletions module/VuFind/tests/integration-tests/src/VuFindTest/Mink/BulkTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,27 +279,54 @@ 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 •
edited
Loading

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_'],
];
}

/**
* 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) {
// For some reason, the click action does not always succeed here; fall
// back to brute force Javascript to prevent intermittent test failures.
echo "\n\nMink click failed; retrying with Javascript!\n";
$session->evaluateScript('$("' . $buttonSelector . '").click()');
$select = $this->findCss($page, '#format');
}
$select->selectOption('EndNote');

// Do the export:
Expand All @@ -312,22 +339,35 @@ 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());
// For some reason, the click action does not always succeed here; fall
// back to brute force Javascript to prevent intermittent test failures.
if (str_starts_with($params, 'lookfor')) {
echo "\n\nMink click failed; retrying with Javascript!\n";
$session->evaluateScript('$("' . $buttonSelector . '").click()');
[, $params] = explode('?', $session->getCurrentUrl());
}
[, $params] = explode('?', $session->getCurrentUrl());
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be removed.

$this->assertEquals(
'print=true&id[]=Solr|testsample1&id[]=Solr|testsample2',
Expand Down