Delete TRX reordering logic from TestingPlatformAutoRegisteredExtensions#7454
Delete TRX reordering logic from TestingPlatformAutoRegisteredExtensions#7454Youssef1313 wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the special-case reordering logic in the MSBuild task that generates the SelfRegisteredExtensions source, so builder hooks are emitted in the order MSBuild provides them.
Changes:
- Removed the well-known TRX hook GUID constant and the
Reorderhelper. - Updated code generation to use
SelfRegisteredExtensionsBuilderHookdirectly (no reordering step).
...Platform/Microsoft.Testing.Platform.MSBuild/Tasks/TestingPlatformAutoRegisteredExtensions.cs
Show resolved
Hide resolved
...Platform/Microsoft.Testing.Platform.MSBuild/Tasks/TestingPlatformAutoRegisteredExtensions.cs
Show resolved
Hide resolved
Evangelink
left a comment
There was a problem hiding this comment.
It definitely was done on purpose but I don't recall the use case. Maybe it's now un-needed because of other refactorings.
@MarcoRossignoli do you recall why it was done?
That's my guess, but I don't have the full history and not sure why it was needed in the past and what changed since then. For history, https://github.com/microsoft/testanywhere/issues/1529 |
|
Hum I wonder if that's linked to the updates to the MessageBus, now we correctly wait for drains so even if TRX is not registered last it can still get all messages. |
|
Because the code coverage has to update the trx with the coverage file, so we need to ensure that the trx is already on disk when the coverage end session kick it from the out of process extension. |
|
@MarcoRossignoli Hmm, that's a good point. I wonder if we are actually already broken today. We have two modes for TRX, one that runs in-proc and one that runs out-of-proc. We run TRX out-of-proc as a TestHostController extension only if That means:
That's just a theory and I need to make more testing to validate what's broken and what's not broken. But generally speaking, that will be problematic for almost any reporter extension. So I wonder if we should do one or both of the following:
|
At least for MS CC, it's the TestHost that publishes the file artifact. There is some IPC that goes between the TestHost and TestHostController where the TestHost asks the TestHostController "give me the artifacts", and then TestHost itself publishes. I need to validate the opposite case then, where both CC and TRX are running out-of-proc |
I cannot see a reason why we need this reordering.