Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nohwnd opened this issue Oct 11, 2024 · 1 comment
Open

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

nohwnd opened this issue Oct 11, 2024 · 1 comment
Labels

Comments

@nohwnd
Copy link
Member

nohwnd commented Oct 11, 2024

In #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 ( #3932 (comment) )

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants