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

Marking tests as CI-Unfriendly #22117

Closed
willdean opened this issue Jun 4, 2017 · 2 comments
Closed

Marking tests as CI-Unfriendly #22117

willdean opened this issue Jun 4, 2017 · 2 comments
Milestone

Comments

@willdean
Copy link
Contributor

willdean commented Jun 4, 2017

As part of an effort to improve coverage on System.IO.SerialPort without compromising the reliability of the CI for everybody, I would like to mark a number of tests as being 'CI Unfriendly' - i.e. we don't want to run them at all, on either inner- or outer- loop. This is because they have behaviour which is inherently timing sensitive and hence not reliable on the CI. Past experience shows they'll often run OK on a PR and then fail (intermittently) later on.

I would like these tests to run by default if you run the suite locally (the suite already takes care of what your hardware capabilities are) but not ever to run on the CI. They're not 'manual tests' per se - they don't need human interaction to run - they just shouldn't run on the CI.

Having looked through CoreFx as best I can, I'm not sure there's a standard approach to this:

  • There are a load of console tests which have [ConditionalFact(nameof(ManualTestsEnabled))] and then define ManualTestsEnabled as checking for a 'Run Manual Tests' environment variable - this is sort of backwards to what we want.
  • Some tests tests have been annotated with a Skip attribute with a note about being manual - that doesn't seem quite right either.

All our tests already have ConditionalFact attributes on with a load of custom logic behind them, so it's easy to add another condition to the relevant ones.

I know people will be thinking:

  • "Just fix the darn tests" - this is hard because they're actually checking real-time timeout behaviour on hardware, which is never going to be a proper pure timing-independent test.
  • "Don't reduce the test coverage" - This is to misunderstand the current situation which is that we've prevented almost all the tests from running on the CI, and that's causing it to miss certain kinds of regression.

Alternatively I could just tag them all with an issue number (probably #20149) and leave it at that. A certain amount of that has happened already and it could be extended.

What is the right approach here? Is there a good technique to determine 'Is running on CI', or does that idea fill people with horror?

@stephentoub
Copy link
Member

@willdean
Copy link
Contributor Author

willdean commented Jun 4, 2017

@stephentoub Perfect, thanks.

For the quick benefit of anyone who comes behind me, it's this attribute:

[Trait(XunitConstants.Category, XunitConstants.IgnoreForCI)]

@willdean willdean closed this as completed Jun 4, 2017
@willdean willdean reopened this Jun 7, 2017
@willdean willdean closed this as completed Jun 7, 2017
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants