Skip to content

Conversation

@agehrke
Copy link

@agehrke agehrke commented Mar 25, 2019

In the current TRX logger implementation, traits on TestCase objects are mostly ignored when converting to the internal ITestElement/UnitTestElement representation.

This PR adds a TestPropertyItemCollection TestProperties property on TestElement class which is populated when converting a TestCase in Converter.ToTestElement() method. TestElement.Save() method is then extended to store the test properties (traits) as Key/Value elements under a Properties element - this follows the XML schema from vstst.xsd.

Example of trx output with traits:

  <TestDefinitions>
    <UnitTest name="TestCaseWithTraits" storage="dummysourcefilename" id="0e9d09de-c08c-235d-bc82-88a161ff28aa">
      <Properties>
        <Property>
          <Key>Trait1</Key>
          <Value>Value1</Value>
        </Property>
        <Property>
          <Key>Trait2</Key>
          <Value>Value2</Value>
        </Property>
      </Properties>
      <Execution id="3612b533-5190-45e7-a28b-96a584ade89d" />
      <TestMethod codeBase="DummySourceFileName" adapterTypeName="some://uri/" className="DummySourceFileName" name="TestCaseWithTraits" />
    </UnitTest>
  </TestDefinitions>

Most of this work is based on @dotMorten's PR #1801.

Tests

I have implemented a test for converting a TestCase with traits, and a test for validating the TRX output of a test with traits.

@msftclas
Copy link

msftclas commented Mar 25, 2019

CLA assistant check
All CLA requirements met.

@agehrke
Copy link
Author

agehrke commented Mar 26, 2019

It seems the test ProcessRequestsShouldProcessMessagesUntilSessionCompleted fails on build agent. It works locally and I haven't touched anything in that area.

@singhsarab singhsarab closed this May 8, 2019
@singhsarab singhsarab reopened this May 8, 2019
@agehrke
Copy link
Author

agehrke commented May 21, 2019

@mayankbansal018 can I do something to make it easier for you to review this PR?

@mayankbansal018
Copy link
Contributor

@agehrke I've been a bit busy with a few thing, will I'll look into this over the weekend

@agehrke
Copy link
Author

agehrke commented May 23, 2019

No rush, I completely understand that you have other more important tasks 🙂

{
return false;
}
return String.Equals(this.key, otherItem.key, StringComparison.OrdinalIgnoreCase) && String.Equals(this.value, otherItem.value, StringComparison.Ordinal);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mayankbansal018
Copy link
Contributor

@agehrke The changes look good, but have we validated that the trx generated after these changes still opens correctly in VS?

@singhsarab
Copy link
Contributor

@mayankbansal018 Please follow up on this.

@Evangelink
Copy link
Member

Hi @agehrke, I know the PR is quite old but would you be ready to fix the conflicts? I will try to do the follow-up.

@agehrke
Copy link
Author

agehrke commented Jan 29, 2022

@Evangelink Yes, of course. I will rebase PR on main branch this weekend and push it. Thanks

@agehrke
Copy link
Author

agehrke commented Jan 30, 2022

I have rebased on main and adjusted changes to new code guidelines (field names, file scoped namespace, etc)

@agehrke
Copy link
Author

agehrke commented Feb 4, 2022

@Evangelink Let me know if you need something more from me.

@Evangelink
Copy link
Member

Hi @agehrke,

Sorry for the delay! I am new to the team and it seems like I went ahead a little too quick here :(. We reviewed the change internally and we decided to add versioning to TRX, before merging this PR. This way users can check if the properties you are adding should be present in the file or not.

I'd like to still say a big thank you for the contribution and for the quick update!

@Evangelink
Copy link
Member

Hi @agehrke,

It's unlikely we will move forward with this PR so I will close it.

Once again thank you for the contribution.

@Evangelink Evangelink closed this Dec 5, 2022
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.

5 participants