Skip to content

Conversation

@GitHamo
Copy link
Contributor

@GitHamo GitHamo commented Feb 6, 2026

Sister PR #10588

@GitHamo GitHamo added the bugfix label Feb 6, 2026
@mjansenDatabay
Copy link
Contributor

@dsstrassner / @kergomard / @thojou / @GitHamo

We (the "Technical Board") are not sure who feels responsible for this. The file is located in the "Soap" component but addresses the domain of "Test".

@GitHamo
Copy link
Contributor Author

GitHamo commented Feb 12, 2026

@mjansenDatabay The changes are in SOAP, so @sKarki999 & I are the responsible.

@GitHamo GitHamo requested a review from sKarki999 February 12, 2026 13:01
@kergomard
Copy link
Contributor

kergomard commented Feb 12, 2026

Hi all

The problem posed by @mjansenDatabay is very real. Right now the test has no surface you could really use to access it for soap, so you are directly accessing internals of the test. I would see this class currently as an adapter class, that SOAP uses to do so and that this adapter can be seen as part of SOAP. This construct is extremely fragile though. We do try to keep this here in mind when refactoring and we see it as necessary to provide a surface through actions to provide an interface to the test in the future.

You should get rid of all direct accesses to database tables of the test in this class as quickly as possible, though. We will update our database tables and fields without looking left or right as this can be seen as a purely internal matter (I know, it wasn't you who introduced this code) and as it is a huge amount of work to find these accesses.

You will also see that in the constructor the TestDIC is used. This is a clear sign that there is something wrong here, as this initialization container is purely internal to the test (otherwise the corresponding interface would be accessible through the $DIC or the mechanism in Components).

These are all just hints. For the time being, I'm very happy, if you see yourselves responsible for this.

Best,
@kergomard

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.

5 participants