#388: Fix more compiler warnings

Very cool, thanks for that work!

Is there a way to integrate checking for warnings into the NMI test
building? I don't want to generally turn compiler warnings into
errors as they can appear quite suddently. But having a way to notice
and flag them would be nice, perhaps is a separate NMI mail that just
greps through the output for any warnings?

Robin

Is there a way to integrate checking for warnings into the NMI test
building?

Yeah, I think that warnings will always be printed on stderr, so we could treat any output there as a build "error". Or we could also grep for "warning" case insensitive in the stdout from make like you said. I'll create a ticket assigned to Daniel for adding that.

- Jon

Is there a way to integrate checking for warnings into the NMI test
building? I don't want to generally turn compiler warnings into
errors as they can appear quite suddently. But having a way to notice
and flag them would be nice, perhaps is a separate NMI mail that just
greps through the output for any warnings?

One question would be whether it's acceptable to add a configure option that compiles with -Werror and then do the NMI builds with that.....

cu
Gregor

One question would be whether it's acceptable to add a configure option
that compiles with -Werror and then do the NMI builds with that….

I think what we want is for the build to complete even if there's warnings so that the tests can still run. i.e. A given NMI job submission should tell us the most information as we can get from it.

- Jon

Could we turn a rebuild of the source code with the -Werror option into a btest case, rather than actually using it for the initial build?

--Gilbert

Could we turn a rebuild of the source code with the -Werror option into
a btest case, rather than actually using it for the initial build?

That's a neat idea, however, any use of -Werror is going to abort the build prematurely, right? So we'll only be capturing the details of the first instance of a warning being emitted if we rely on -Werror, when I think it's more useful if NMI could give us all the warnings that were emitted.

But I'm still open to adding an additional btest that uses -Werror that would be useful at getting developers to think twice about filing a merge request for code that breaks tests because of warnings.

- Jon

True. How about redirecting make output to a log and running a script against the output? Any warning: output fails the test, but we still get the entirety of the output this way.

--Gilbert

True. How about redirecting make output to a log and running a script
against the output? Any warning: output fails the test, but we still
get the entirety of the output this way.

Yeah, that's the way I was imagining. Does anyone like the idea of this being a regular btest versus something that only runs on NMI? I'm not sure how much benefit is derived from the former at the expense of increasing the testing time by a few minutes.

+Jon

I think that makes sense. We'd at least get a chance to see if we introduces any warnings locally before committing where they may be exposed on other platforms on NMI.

  .Seth

Checking locally is good, but I'm also concerned about increasing
testing time for every btest run. How about making it an option? We
can use @test-requires to test some preconditionn, like a set
environment variable.

Alternatively, I have a btest patch pending that introduces test
groups. With that, we could make a group "long-tests" for tests that
take a while to run. btest would then just need another option to not
run certain groups by default (the patch already has an option to
*only* run certain groups).

Robin