[TASK] Make getInstanceIdentifier() and getInstancePath() non-static for overridability

[dkd-kaehm]

Convert protected static methods to protected instance methods and mark as @internal.
This allows subclasses to override these methods for proper test isolation in parallel
execution (Paratest), without making them part of the public API.

Instead of using static:: binding, use $this-> for natural method overriding.
Solves the need for test isolation in single-test-level parallelization while
signaling that these are internal implementation details.

Fixes: #708

[bmack]

Hey @dkd-kaehm ,

please either add a test or a doc comment, so this change will stay around and not break when touching these lines again.

[dkd-kaehm]

Hey @dkd-kaehm ,

please either add a test or a doc comment, so this change will stay around and not break when touching these lines again.

both done, please kick the GH Actions.

[lolli42]

Mhh. First thought: "Making this de-facto API will probably add headaches as soon as we pick up the 'composer based functional tests' works again." Second thought: "Why are these two methods static anyway?"

-> I understand your need to override these methods when you want to parallelize on a single-test level. I personally would try to avoid that, which is why core parallelizes functionals on a test-class level instead. To still solve your request, we could maybe: Make the methods non-static, keep protected and not private, but mark @internal. This would solve the binding issue, still allows you to override, and makes clear: "If you override, you're on your own and it may break if testing-framework decides to continue in a diverging direction". What do you think?

[dkd-kaehm]

Mhh. First thought: "Making this de-facto API will probably add headaches as soon as we pick up the 'composer based functional tests' works again." Second thought: "Why are these two methods static anyway?"

-> I understand your need to override these methods when you want to parallelize on a single-test level. I personally would try to avoid that, which is why core parallelizes functionals on a test-class level instead. To still solve your request, we could maybe: Make the methods non-static, keep protected and not private, but mark @internal. This would solve the binding issue, still allows you to override, and makes clear: "If you override, you're on your own and it may break if testing-framework decides to continue in a diverging direction". What do you think?

IMHO it is better, but what is with tests then?

[dkd-kaehm]

@lolli42
switched to non-static.

[lolli42]

Hey Rafael. Sorry for the delay. I had to first fix CI today, and had another look at the change now.
I rebased it and gave it some thoughts.
I'm sorry for discussing so many details on this rather tiny change, but reading it again I'm now actually a bit unsure if people may hurt us by making especially getInstancePath() non-static, this might be considered breaking in seldom cases (and core probably actually stumbles here). I'll try to pull @sbuerk into this to come up with a final conclusion to solve your initial issue, soon. Please be patient, I'm sure we'll find consensus.

[lolli42]

Ok. Had a chat with Stefan on this. We will go with your initial approach (without the test). This is not breaking an risk less. Sorry for the confusion and thanks for your work.