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

[php8-compact] Further test fixes for php8 #20597

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

seamuslee001
Copy link
Contributor

Overview

Fixes the following 2 issues

  • CRM_Core_InvokeTest::testInvokeDashboardForNonAdmin
    Undefined array key "beginHookFormElements"

/home/jenkins/bknix-edge/build/build-0/web/sites/default/files/civicrm/templates_c/en_US/%%4D/4DC/4DC76B26%%body.tpl.php:36

  • CRM_Contact_Import_Form_DataSourceTest::testBuildForm
    Required parameter $attributes follows optional parameter $from

/home/jenkins/bknix-edge/build/build-0/web/sites/all/modules/civicrm/CRM/Core/Form/Date.php:164

Before

Those 2 tests fail on php8

After

Those 2 tests pass on php8

ping @eileenmcnaughton @demeritcowboy @totten @colemanw

@civibot
Copy link

civibot bot commented Jun 14, 2021

(Standard links)

@civibot civibot bot added the master label Jun 14, 2021
@@ -32,7 +32,7 @@
{/if}

{* Add all the form elements sent in by the hook - formerly used by civiDiscount, now deprecated*}
{if $beginHookFormElements}
{if !empty($beginHookFormElements)}
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to suggest that as per the code comment to maybe do something else, but the code comment isn't right anymore since the deprecation was reverted in civicrm/org.civicrm.module.cividiscount#254. And also it looks like it's used in a small handful of other extensions.

@demeritcowboy demeritcowboy merged commit afcc829 into civicrm:master Jun 14, 2021
@eileenmcnaughton eileenmcnaughton deleted the php8_test_fixes branch June 14, 2021 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants