Add flaky test attribute #8811
Add flaky test attribute #8811
Conversation
davidwengier
left a comment
davidwengier
left a comment
There was a problem hiding this comment.
I love the idea, but I'm not sure on the terminology. "Flaky" feels too much like an acceptance of guilt 😛
Can we use "RAZOR_RUN_ALL_TESTS"? or "RAZOR_RUN_SKIPPED_TESTS" and call the attribute "ConditionalSkip" or something? Actually, isn't there a [ConditionalFact] in xunit? I'm sure I've seen it somewhere
src/Razor/test/Microsoft.VisualStudio.Razor.IntegrationTests/FlakyFactAttribute.cs
Outdated
Show resolved
Hide resolved
| "[json]": { | ||
| "editor.tabSize": 2 | ||
| }, | ||
| "dotnet.defaultSolution": "Razor.sln", |
| - name: RAZOR_RUN_FLAKY_TESTS | ||
| value: 'true' |
There was a problem hiding this comment.
I thought you were talking about having a separate pipeline to run all tests? Seems like we're just guaranteeing failures if we turn this on here, which means guaranteeding people will have to investigate them, but we already have issues for these.
I would expect either a different pipeline for all tests, so we can compare the two, or use the FlakyFact attribute only on tests that we know for sure will at least pass some % of the time.
There was a problem hiding this comment.
I'm open for either.
I would expect either a different pipeline for all tests, so we can compare the two, or use the FlakyFact attribute only on tests that we know for sure will at least pass some % of the time.
This was my initial thought. Running the test pipeline and having failure rates generated is worth it. If we want a separate pipeline that's fine too, but I don't much see the point. I figure Conditional Skip = may be enabled at some point. It's the difference between "might work" and "definitely won't work". 0% success rate should just be disabled or fixed.
There was a problem hiding this comment.
I guess I agree with you, I'm just not confident that our existing skipped tests are all there because of flakiness, and not some other issue. We'll find out! 😀
There was a problem hiding this comment.
Well, we're off to a good start!
Hmm... maybe you're thinking of https://sourceroslyn.io/#Microsoft.CodeAnalysis.Test.Utilities/Assert/ConditionalFactAttribute.cs,aec721ce5b3203aa ? We definitely could just do the same thing |
Probably where I saw it, but probably overkill to use here. |
2 participants
Add an attribute that will run tests if the RAZOR_RUN_FLAKY_TESTS is set, otherwise they will be skipped.