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

CaseActivityTest - Fix quiet regressions #25416

Merged
merged 2 commits into from
Jan 25, 2023

Conversation

totten
Copy link
Member

@totten totten commented Jan 25, 2023

Overview

After eb92dd7, CaseActivityTest stopped running reliably. After 98e528a, one of the assertions started failing.

Depends: #25415

Before

Test does not run consistently.

After

Test runs and passes.

Comments

More details in commit notes

@civibot
Copy link

civibot bot commented Jan 25, 2023

(Standard links)

@civibot civibot bot added the master label Jan 25, 2023
Following eb92dd7, the `CaseActivityTest` started to run
only intermittently. Why?

__high-level__: `Civi\Core\ClassScanner` and `phpunit8` both do a scan over the folder `tests/phpunit/CRM/Case/WorkflowMessage`

__low-level__: `Civi\Core\ClassScanner` has caching. Depending on the state of the cache, it may or may not do a scan:

* If the cache is filled, then `ClassScanner` doesn't need to scan.
    * When `phpunit8` subsequently does a scan, it will load `CaseActivityTest.php` normally.
* If the cache is empty, then `ClassScanner` does the first scan. It is the one that actually loads `CaseActivityTest.php`.
    * Later, `phpunit8` does a scan. Due to a quirk, it doesn't realize the class exists.

The scanner in phpunit works roughly like this:

```php
$tests = [];
foreach (glob('*Test.php') as $file) {
  $before = get_declared_classes();
  require_once $file;
  $after = get_declared_classes();
  $tests = array_merge($tests, array_diff($before, $after));
}
```

So if the class was previously loaded, then phpunit doesn't see it.
The relevant example data was changed by 98e528a.
@totten
Copy link
Member Author

totten commented Jan 25, 2023

civibot, test this please

@eileenmcnaughton eileenmcnaughton merged commit 4502008 into civicrm:master Jan 25, 2023
@totten totten deleted the master-scan-fix branch January 25, 2023 10:09
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