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 unit test to hit class-loader issue #24712

Closed
wants to merge 1 commit into from

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 11, 2022

Overview

Add unit test to hit class-loader issue

Before

I hit an issue enabling civiimport on master / the rc where it failed on a class not found when an import already exists (it registers the import subscriber to create entities so having already done an import matters here).

When I tried to replicate in a test it got weird fast. Locally this is now failing on not finding an afform class after running this test

After

What changed? What is new old user-interface or technical-contract?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

Technical Details

This is the backtrace & it fails on the afform extension currenlty
image

image

image

Comments

Anything else you would like the reviewer to note

@civibot
Copy link

civibot bot commented Oct 11, 2022

(Standard links)

@colemanw
Copy link
Member

retest this please

@demeritcowboy
Copy link
Contributor

Does this extension do something like create dynamic php classes? In a normal user-scenario you would enable the extension, then another page run would happen where class-scanning picks up whatever. But here in a script, class-scanning maybe doesn't have a chance to do another run after you've enabled the extension. I'm guessing a bit but the error is that it can't map Import_1 to a class. If the extension is already pre-enabled before the tests are run then it finds it. I don't see an obvious way to refresh class-scanning on-demand, assuming that's the problem.

@totten
Copy link
Member

totten commented Oct 14, 2022

I spent a few hours debugging this on the flight home. I'm pretty sure the failure in this test arises like so:

  • It enables the new extension
  • It clears lots of caches
  • But it doesn't clear the index of classes.
  • Therefore, it doesn't detect that civiimport's ImportSubscriber exists.
  • Therefore, it doesn't detect that ImportSubscriber is subscribed to civi.api4.entityTypes

The patches in 24743 and 24744 seemed to fix this test on my laptop.

(I have no idea if this test/situation is representative of any other anecdotal failures about class-loading.)

@eileenmcnaughton
Copy link
Contributor Author

This has been absorbed into other PRs....

@eileenmcnaughton eileenmcnaughton deleted the import_load branch October 20, 2022 03:28
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.

4 participants