Introduce strategies for test ID generation#1306
Conversation
2f839f9 to
1e0ba6a
Compare
src/Adapter/MSTest.TestAdapter/Extensions/TestCaseExtensions.cs
Outdated
Show resolved
Hide resolved
1e0ba6a to
3e82f7a
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3e82f7a to
469ddd5
Compare
6e0148c to
3ab0011
Compare
Haplois
left a comment
There was a problem hiding this comment.
How is this going? Would you like any help @Evangelink?
|
Hey thanks for asking @Haplois. Looks like I just need to fix CI tests (they are passing locally for me). There are still some tickets open (not fixed by this change) that I need to investigate but I feel confident we will be able to merge this one soon(TM). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3ab0011 to
d349ee9
Compare
Evangelink
left a comment
There was a problem hiding this comment.
@Haplois please feel free to do a review when you have some free time.
src/TestFramework/TestFramework/Attributes/DataSource/TestIdGenerationStrategy.cs
Show resolved
Hide resolved
src/TestFramework/TestFramework/Attributes/DataSource/TestIdGenerationStrategy.cs
Show resolved
Hide resolved
src/Adapter/MSTest.TestAdapter/Extensions/TestCaseExtensions.cs
Outdated
Show resolved
Hide resolved
src/Adapter/MSTestAdapter.PlatformServices/PublicAPI/PublicAPI.Unshipped.txt
Show resolved
Hide resolved
test/E2ETests/DiscoveryAndExecutionTests/TestId.DefaultStrategy.cs
Outdated
Show resolved
Hide resolved
|
Thanks for the review guys! @Haplois What do you mean by the bail-out strategy? |
|
See my comment above on the code. |
src/TestFramework/TestFramework/Attributes/DataSource/TestIdGenerationStrategyAttribute.cs
Show resolved
Hide resolved
6896179
|
@MarcoRossignoli @Haplois I have addressed all comments, if you want to have another look. @Haplois There is one open question remaining for you. |
|
@Evangelink uploaded a patch file for the broken ID gen. Can't push to this branch anymore. |
|
@Haplois I have applied and updated your patch. Let me know if you have any other comment or if I can move forward with merging the PR. |
Fixes #1286 and fixes AB#1634557
TODO: