Deciding on policy about using filters or [SkippableFact] with CI = true env var for skipping tests

[tkiapril]

There are two major workarounds in skipping tests for locally failing tests on CI:

1. Using filters in CI script to skip tests (this is what we've doing until now)
2. Marking tests with [SkippableFact] from Xunit.SkippableFact and testing if CI = true environment variable is set (CircleCI, Github Actions)
   • We can also extend [SkippableFact] and make an attribute that makes tests marked with it skipped when a particular environment variable is set.

[dahlia]

The problem with the first approach is that contributors are hardly aware of its place and even its existence. For that reason, I prefer the second approach over the first one.

[OnedgeLee]

Actually I'm not sure if it's really better to use environment variable on CI.
I think test code have to be independent to CI, and CI have to be dependent to test code.
Our tests are categorized to (linux, window, macos), and (netcore, unity),
and I think each test must have options to be skipped or not for each categories.
If we put away unity test, the categories are just a matter of OS, and for this, I think 2nd option is better.
But when it comes to cover unity test, I think it's a bit weird that test code have to know whether it's executed on netcore test or unity test.

[OnedgeLee]

If we select unity test with allowed list method, I think 2nd option is better, but I think it's just proposed and not clearly decided yet.

[dahlia]

I'd rather find environment variable CI a kind of standardized system variables like PATH and HOME. Actually, our tests already depend on several environment variables, and some of them are even custom-defined by us.

[OnedgeLee]

To exclude some tests just on unity test, don't we have to use environment variable other than CI, since some of methods malfunction on unity?
What I've thought weird was about that, but after careful thinking, my mind has been changed.
Actually libplanet is also have to be used under unity condition, and it seems not so much weird that it has environment variable for unity condition. May be 2nd option would be better.

[tkiapril]

Now that I think of the use of environment variables to skip particular tests, I feel that it's similar to using #IFDEF macros in C to determine the environment. That is: if the test would only run in cases where an environment variable is set, it may be problematic, but if is only ignored when a environment variable is set, it wouldn't be bad as much. On the other thought, we could define something like [ConditionallySkippableFact(IfEnvSet="env_var_name")] to make clear in the codebase that it's intended to be skipped if a particular environment variable is set.