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

(NFC) Skip CliRunnerTest on php80+drush+Backdrop #23184

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

totten
Copy link
Member

@totten totten commented Apr 13, 2022

Overview

The CliRunnerTest goes through various permutations of CLI commands (cv, drush, wp-cli) in different environments (php7/min, php8/max, D7, D9, BD, WP, etc). The permutation for php8+backdrop+drush has been failing persistently (https://test.civicrm.org/job/CiviCRM-E2E-Matrix/).

Skip this test permutation. It is blocked by upstream issues in drush-backdrop, and (AFAIK) this permutation has never worked.

CC @seamuslee001 @herbdool

Before

These tests fail in php8+backdrop+drush:

E2E_Extern_CliRunnerTest.testBasicPathUrl with data set "drush"
E2E_Extern_CliRunnerTest.testPathUrlMatch with data set #0
E2E_Extern_CliRunnerTest.testPathUrlMatch with data set #1

After

The tests are skipped in php8+backdrop+drush.

Technical Details

The problem does not seem to be specific to the functionality civicrm-core.git -- it appears to affect anything going through drush-backdrop on php8, and it's been blocked on upstream fixes. You can see this by running just drush ev:

$ drush ev 'echo 123;'
PDOException: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens: SELECT name FROM {system} WHERE type='module' AND status=1; Array
(
    [:type] => module
    [:status] => 1
)
 in drush_db_select() (line 131 of phar:///Users/totten/bknix/extern/drush8.phar/includes/dbtng.inc).
Drush command terminated abnormally due to an unrecoverable error.

I'm not too worried about losing coverage here - CliRunnerTest still provides coverage for several adjacent configurations, ie

  • PHP8 + D7 + Drush (differ only by CMS)
  • PHP8 + Backdrop + Cv (differ only by CLI-runner)
  • PHP7 + Backdrop + Drush (differ only by PHP-version)

Comments

It's hard to see this when skimming the matrix (https://test.civicrm.org/job/CiviCRM-E2E-Matrix/), but there is an additional error for backdrop,max regarding SoapTest. I haven't been able to reproduce this yet. I mention it only to show why these extra red-flags matter -- when we get multiple+persistent red-flags for something outside our control, we're less likely to see other red-flags.

When/if there's an upstream fix, we can re-enable the test...

There seems to be a problem in using `drush ev` with our current versions of Drush / Backdrop / PHP 8.0

```
[bknix-edge:~/bknix/build/bcmaster/web/modules/civicrm] drush ev 'echo 123;'
PDOException: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens: SELECT name FROM {system} WHERE type=&civicrm#39;module&civicrm#39; AND status=1; Array
(
    [:type] => module
    [:status] => 1
)
 in drush_db_select() (line 131 of phar:///Users/totten/bknix/extern/drush8.phar/includes/dbtng.inc).
Drush command terminated abnormally due to an unrecoverable error.
```

The problem is not specific to the functionality `civicrm-core.git` -- it
affects anything going through `drush-backdrop` and seems to be blocked on
upstream fixes.  But it's been generating a bunch of red-dot noise for the
past few months, and there's no point in that.

Note that `CliRunnerTest` still provides coverage for several adjacent combinations, so I'm not
too concerned about substantive change affecting civicrm-core. Those adjacent combinations are:

* PHP80 + D7 + Drush (differ only by CMS)
* PHP80 + Backdrop + Cv (differ only by CLI-runner)
* PHP7x + Bacdkrop + Drush (differ only by PHP-version)
@civibot
Copy link

civibot bot commented Apr 13, 2022

(Standard links)

@civibot civibot bot added the master label Apr 13, 2022
@totten totten changed the title Backdrop - Skip CliRunnerTest on php80+drush Skip CliRunnerTest on php80+drush+Backdrop Apr 14, 2022
@totten totten changed the title Skip CliRunnerTest on php80+drush+Backdrop (NFC) Skip CliRunnerTest on php80+drush+Backdrop Apr 14, 2022
@totten
Copy link
Member Author

totten commented Apr 14, 2022

Changed to "NFC" because (1) it only changes the test and (2) the test-scenario never passed.

@seamuslee001 seamuslee001 merged commit e99c7a4 into civicrm:master Apr 15, 2022
@totten totten deleted the master-bd-drush branch April 15, 2022 22:47
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