Add --warnings-as-errors flag for non-zero exit code (Take 2)

[eksperimental]

Closes #1411 #1294

PR Originally published in #1412 #1564

[eksperimental]

There's one thing that is left. The error message says the docs failed to build due to warnings, which is not true. The docs are generated:
I think docs should not be generated. If that is the case we would have a temporarily build them in a folder inside the output folder, and on failure remove it..
Let me know what you guys think.

Also, what should the behavior be given mix docs uses generates epub and html, and one fail? Should none of them be created, or should they be judged independently.

[josevalim]

We should definitely produce a build, because it allows users to introspect what was rendered even in the presence of warnings.

[eksperimental]

We should definitely produce a build, because it allows users to introspect what was rendered even in the presence of warnings.

Great. That simplifies things.

There's is one issue though:

mix docs --warnings-as-errors generates the HTML docs and dies due to the error, and EPUB docs are never generated.

[warmwaffles]

Producing at least 1 build is fine. I believe it would be easier to inspect the HTML version first anyways. You could also specify which format you wanted if you really needed the epub to inspect.

[eksperimental]

Producing at least 1 build is fine. I believe it would be easier to inspect the HTML version first anyways. You could also specify which format you wanted if you really needed the epub to inspect.

We need to standardize this (ie. be consistent) since other formatters could be used. I am working on a fix that will generate everything, and error if it has to, at the end.

[wojtekmach]

we are checking for warnings without generating the docs. I believe this should rather be something like this:

if List.keymember?(opts, :version, 0) do
  IO.puts("ExDoc v#{ExDoc.version()}")
else
  generate(args, opts, generator)

  if opts[:warnings_as_errors] == true and ExDoc.Utils.warned?() do
    # ...
  end
end

we need to improve cli_test.exs because it's not catching this bug.

[eksperimental]

I have noticed this. I am sending a fix later today that addresses this and the issue regarding all docs being generated and failing after that.

[wojtekmach]

as mentioned above there is a bug that this test is not catching. what about making it more realistic, have an actual .ex or .md file, and assert on the warning message?

[eksperimental]

@wojtekmach I have submitted new changes. Could you please have a look.

[eksperimental]

There's a little issue with two lines of output generated during the test run, but we can talk about it later on before merging the PR.

[eksperimental]

Is there any reason why this is being held back?

[josevalim]

Sorry, I was not aware this was ready to go.

I would like to propose two changes:

1. Introduce run_with_io for the cases you care about io. run can just call run_with_io. This should reduce the changes in this PR considerably.

2. Fixtures get really unmaintainable over time and our goal is to discard all fixtures in the future. Is there a way we can test this feature without introducing them?

[josevalim]

Thank you!

[eksperimental]

@josevalim which module are you referring to about introducing run_with_io.

[eksperimental]

Any other than Mix.Tasks.DocsTest?

[josevalim]

All the modules that need it, yes. The goal is to minimize the changes in the PR by not changing the return type of run.

[eksperimental]

I get it now. It was about the return type.

[eksperimental]

Code have been changed to make the least changes.

Regarding fixtures, I played around with them for an hour to no avail. If you have a clear idea of how to test this I can try. If not once there is a way of porting what's in a fixture to run without it I can take care of adapting the code.