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

Occasional Errors In Pipeline #316

Open
dmanners opened this issue May 7, 2024 · 8 comments
Open

Occasional Errors In Pipeline #316

dmanners opened this issue May 7, 2024 · 8 comments

Comments

@dmanners
Copy link

dmanners commented May 7, 2024

While running phpstan in the context of our Magento project locally we get no errors, but occasionally with the same code base running in a ci/cd pipeline we will get errors that will either change or disappear on a re-run.

The errors that do appear seem to only occur with extension attributes.

71 Call to method getSuppliers() on an unknown class
Magento\Sales\Api\Data\OrderExtensionInterface.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols

In our pipeline and also lokally we are running

./vendor/bin/phpstan analyse --memory-limit=4G --configuration=.dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/db-systel.neon

With the config

parameters:
	level: 4
	treatPhpDocTypesAsCertain: false
	paths:
		- %currentWorkingDirectory%/app/code
		- %currentWorkingDirectory%/app/design
	magento:
		magentoRoot: %currentWorkingDirectory%
	ignoreErrors:
		-
			# reason: false positive magic methods
			message: '#Call to an undefined method [a-zA-Z0-9\\_]+::setData|hasData|unsetData|getData|toArray\(\)#'
		-
			# reason: false positive with CartInterface
			message: '#Call to an undefined method Magento\\Quote\\Api\\Data\\CartInterface::getAllVisibleItems\(\).#'
		-
			message: '#Call to an undefined method Magento\\Quote\\Api\\Data\\CartInterface::getGrandTotal\(\).#'
		-
			message: '#Call to an undefined method Magento\\Quote\\Api\\Data\\CartInterface::getPayment\(\).#'

I have tried clearing the phpstan cache both locally and in the pipeline as well as locally clearing the generated folder as in the pipeline we have no generated folder.

@shochdoerfer
Copy link
Member

Thanks for reporting the issue @dmanners.

The difference between local and CI runs is that locally, PHPStan and this extension will most likely use the Magento-generated classes (if they exist), while in CI, the PHPStan extension generates the missing classes by itself with its own logic. Still, that does not explain why CI occasionally breaks.

@dmanners
Copy link
Author

dmanners commented May 7, 2024

Yeah it's odd some runs it's fine and some the errors appear but on a rerun everything is then green. I have also tested it locally without a generated folder and it's still green locally. I will also check that we dont override a magento file with a local patch that isnt applied in the pipeline as well.

I have also tested out firstly clearing the phpstan cache folder before running the check.

@dmanners
Copy link
Author

dmanners commented May 7, 2024

Ok after a bit more debugging I have found that disabling the parallel processing then the errors do not seem to appear.

	parallel:
		maximumNumberOfProcesses: 1

Here I have run our pipeline 3 times and had no error cases.

@shochdoerfer
Copy link
Member

Uh, good catch! Fixing this issue might not be easy due to the way the autoloader & the code generation in this extension work. Will think about it.

@dmanners
Copy link
Author

dmanners commented May 7, 2024

I will keep this updated after a week or so of pipeline runs to see if this setting helps us in the long run or just randomly today.

@shochdoerfer
Copy link
Member

Sounds like a plan. Don't close the issue if the current setting works for you. I'll take this as an opportunity to discuss with Ondrej how to better hook into PHPStan to generate the classes when they do not exist. The autoloader magic isn't ideal and seems to break a few things, e.g. the Rector integration (see #297). Looks like a larger refactoring is needed to improve things.

If the setting does work for you, please consider adding a PR to the README that includes a hint on how to resolve the issue.

@dmanners
Copy link
Author

Yeah this seems to have fixed the issue with us a week of pipeline runs and no false positive errors in the phpstan job.

@shochdoerfer
Copy link
Member

I've discussed this issue with Ondřej the other day. If you want to run the processes in parallel, it would be best to run Magento's setup:di:compile command before running PHPStan. This way, Magento will generate all "missing" files, and the autoloaders of our extension are not executed.

I will have to check how to change the extension's architecture to allow the code generation to happen only once. Ideally, without invoking a separate CLI tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants