search

microsoft / testfx

MSTest framework and adapter
MIT License
736 stars 254 forks source link

Standard output is provided in two different properties on test node #3933

Open nohwnd opened 1 week ago

nohwnd commented 1 week ago

In https://github.com/microsoft/testfx/pull/3932 we put standard output into two properties on test node, those are potentially both serialized when calling a client, check if that is the case, and make sure that newer TE client can consume the platform provided property so we don't have to send both forever.

See this conversation for details ( https://github.com/microsoft/testfx/pull/3932#discussion_r1797016815 )

nohwnd 1 hour ago These are potentially much larger than the rest of the message, is it good idea to have 2 copies of the text in the object? Is this the only way to do this? can "vstest.TestCase.StandardError" be dropped in favor of the new Platform provided property?

Member @MarcoRossignoli MarcoRossignoli 1 hour ago • We cannot we need 2 way because the SerializableKeyValuePairStringProperty is an internal prop to accomodate VS. VS TE is working in 2 different mode where the values are taken using different key, value. Under VS protocol we declare vstest capablity and that value is the expected one.

Adding StandardErrorProperty like xunit did is to have the new model and avoid the need to go with an untyped "key name" that's also used in the protocol atm as vstest capability. Otherwise the core platform should embed code related to the protocol for the vstest capability.

Member @MarcoRossignoli MarcoRossignoli 1 hour ago My idea was to drop the vstest capability, but looks like at time wasn't good for TE. cc: @drognanar

Member @nohwnd nohwnd 1 hour ago So the solution here is fix the VS client, and then remove the vstest.* property, yeah?

Member @nohwnd nohwnd 52 minutes ago Do you know if we end up serializing both when calling TE or some other client? If this is not the case then the overhead of having two "copies" of the same string on the node is minimal, but I doubt that is the case, and we actually send it twice.

If you don't know off hand just say it , I will make an issue for investigating it.

Evangelink commented 19 hours ago

@nohwnd it's not clear what is needed here so I'll let you do the triaging.