No way to distinguish between `mix test --warnings-as-errors` test failures (exit 2) or test failures with warnings in test files (exit 2)

[naserca]

In our CI pipeline, we rerun failed tests once to pass flaky tests that fail inconsistently. This means the following scenario is quite common:

1. mix test --warnings-as-errors -> exit 2
   1. flaky test failure in foo_test.exs
   2. compilation warning in bar_test.exs but tests pass
2. mix test --failed --warnings-as-errors -> exit 0
   1. flaky foo_test.exs passes
   2. compiler doesn't require bar_test.exs again so the warning is lost

Happy to help with a solution here if folks think the problem is worth solving.

[marcofognog]

I don't think --warnings-as-errors flag is meant to be understood as "warnings as failures". 🤔

From the Mix.Test.Task module docs:

* `--warnings-as-errors` *(since v1.12.0)* - treats compilation warnings (from loading the
  test suite) as errors and returns a non-zero exit status if the test suite would otherwise
  pass. Note that failures reported by `--warnings-as-errors` cannot be retried with the
  `--failed` flag.
  This option only applies to test files. To treat warnings as errors during compilation and
  during tests, run:
      MIX_ENV=test mix do compile --warnings-as-errors + test --warnings-as-errors

So strictly speaking, the behavior you described works as expected.

Now, trying to rephrase the problem: --warnings-as-errors was introduced to make builds fail when all tests pass and have warnings, like a safety net.
But if your build fails tests with no warnings, you retry with --failed, the warnings-as-errors flag doesn't fulfill its purpose anymore.

Should we think about a warnings-as-failures flag?

[josevalim]

Ping @naserca! I remember we discussed more about this issue. Can you remind me which conclusion we landed? Are you still planning to tackle this?

[naserca]

:( Sorry @josevalim! I’ve been away from a keyboard for a few weeks. I do plan to pick this up when back next week. General plan is (sorry on my phone!):

1. On each test run (without --failed or --stale, we cache compilation warnings emitted from test files in a new manifest file.
2. On a test run with --failed and --all-warnings, we print warnings from test files even if the test files are excluded by --failed.
3. If --warnings-as-errors as well, those warnings from excluded test files will actually result in an error.

Given the cache, I’ll have to deal with touched files in between runs, but I benchmarked loading all excluded files on each --failed run, and in our typical CI workflow, this was prohibitively slow. I think we decided this is the least bad approach. :)

[naserca]

@josevalim I don't see any better options here than exposing the ability to properly distinguish between "warnings and test failures" from "just test failures," so I opened this quick MR #14405

This idea for example leaves us open to this case:

1. mix test test/with_warnings_test.exs test/with_failures_test.exs
2. [cache warnings from test/with_warnings_test.exs files]
3. mix test --failed
4. [we still have to load all the other test files in the entire project]

Wanted to at least offer some solution.

[josevalim]

Closing in favor of PR.