Skip to content

Conversation

@Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Nov 4, 2025

Related to #3933

@Evangelink Evangelink merged commit 5c6ea3b into main Nov 4, 2025
9 checks passed
@Evangelink Evangelink deleted the dev/ygerges/trx-enabled branch November 4, 2025 09:06
@nohwnd
Copy link
Member

nohwnd commented Nov 4, 2025

One more comment here ^^^ even though it is already merged.

It also seems to compose a bit weirdly. Will we be able to remove that interface in the future (e.g. with removal of vstest bridge)? "isEnabled" seems like quite generic thing that we might want to check on more capabilities.

@Youssef1313
Copy link
Member Author

If we removed the bridge, I think we will be able to clean that up a lot.

The main issue here is that the VSTestBridge that implements lots of functionality doesn't know about the individual test frameworks. The interface design of ITrxReportCapability makes it worse. You need to concrete implementation to understand whether or not it's enabled. What I did here is introducing an extra internal interface that provides the additional information that the concrete implementations know of.

Ideally, we would just fix ITrxReportCapability. But it's a breaking change and it was rejected for MTP v2.

@nohwnd
Copy link
Member

nohwnd commented Nov 4, 2025

Okay makes sense. I thought it was something like that. I have bunch of such "tricks" in vstest, and I really don't like them. One of the classes is private so no problem there, and the other is public but tied to vstest bridge it seems. So we should not have hard time removing that interface.

@Youssef1313
Copy link
Member Author

So we should not have hard time removing that interface.

We might have an acceptable break because of IVT, if old MSTest is used but VSTestBridge is upgraded.

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

Successfully merging this pull request may close these issues.

4 participants