Fix #25 assertModulesLoaded not matching all loaded Modules

[visto9259]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	no

Description

• Fixes issue assertModulesLoaded not matching all loaded Modules #25

[visto9259]

This is only for #25.
As this is my first code contribution, I will wait for review feeback to ensure the process was followed correctly before proceeding with resolving the other issues.

[visto9259]

I thought I had signoff set in my IDE. I guess I was wrong.

[Xerkus]

You will need to rebase this

[visto9259]

Because of the missing signoff? No problem

[Xerkus]

No, because it is based off of the wrong branch

[visto9259]

I am confused here.
Arent't bug fixes against current release branch? which 4.10.x

[gsteel]

Thanks @visto9259

LGTM other than a couple of minor points.

The remaining 3 UnusedBaselineEntry issues can be solved by running vendor/bin/psalm --update-baseline

I'll leave approval for someone else who has a better memory for MVC stuff 👍

[gsteel]

Suggested change

	/** @var ModuleManager $moduleManager */
	$moduleManager = $this->getApplicationServiceLocator()->get('ModuleManager');
	$moduleManager = $this->getApplicationServiceLocator()->get('ModuleManager');
	assert($moduleManager instanceof ModuleManager);

Try to use an assertion in cases like this - it has the same effect of telling your IDE and psalm what the type is expected to be, but with the added benefit of causing a runtime error when the type does not match with a clear assertion failure.

When the type should be verified as part of expectations for the subject under test, then PHPUnit's self::assertInstanceOf() method should be used.

[visto9259]

Good point. Thanks

[visto9259]

So was I correct in using the 4.10.x branch for a bug fix?

[gsteel]

Looks to me like you worked on the patch by branching from 4.11.x, then sent the changes to the 4.10.x branch, hence all the lock file maintenance commits from renovate that are not present in 4.10.x.

In order to clean things up, you'd need to create a new branch from 4.10.x, then "Cherry Pick" your commits here from "Fix Laminas 25", then force push that new branch to this PR.

[visto9259]

Good grief.... you are right! Rookie mistake.

[Xerkus]

Ignoring DCO check for non-essential commits

[Xerkus]

@visto9259 thank you