+1 2015-02-02 14:12 GMT+01:00 Thomas Mortagne <[email protected]>:

+1

On Mon, Feb 2, 2015 at 10:45 AM, [email protected] <[email protected]> wrote:

...

Hi devs,

I don’t recall if we had already agreed to this proposal or not so now that I’ve progressed with its implementation I’m sending this mail to ensure we all agree.

Rationale:

* The build logs should not be polluted with “normal” stack traces, or any message. Anything that the test produces should be contained and asserted in the test itself. * If some logs go through it means they’ve not been captured and not been asserted in the test. * It makes the test writer more aware of what the code being tested outputs and thus what the test writer needs to do about these.

Implementation details:

* Fail the build if some unit test output content to stdout or stderr. * By default any code that outputs log will send content to stdout (since by default slf4j/logback will use a Console Appender that logs to stdout). * Thus when code output logs, the test need to capture those logs. I’m introduced a new rule (@AllLogRule - see https://gist.github.com/vmassol/d6357901fca25db74783) which captures all logs. * The @AllLogRule rule will fail the test if the captured logs are not asserted (it can be asserted for its content or the test can explicitly say it doesn’t care about it). * There will be a custom RunListener (CaptureConsoleRunListener - see https://gist.github.com/vmassol/f3b7496b5bd3a9693fc2) plugged into the Maven Surefire plugin that fails the build if there’s content sent to stdut/stderr * Our Maven build will perform the check by default and we’ll start by disabling it in the modules that currently fail. * The idea is that slowly over time we remove the disabling in those modules as we fix the tests and that for new modules we apply the new rule (which will be enabled by default)

Example of capturing/asserting logs:

@Rule public AllLogRule logRule = new AllLogRule(); … assertEquals("Error getting resource [bad resource] because of invalid path format. Reason: [invalid url]", this.logRule.getMessage(0));

Example of ignoring a log:

… assertNotNull(this.logRule.getMessage(0));

WDTY?

Thanks -Vincent

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Guillaume Delhumeau ([email protected]) Research & Development Engineer at XWiki SAS Committer on the XWiki.org project

Hi Sergiu, On 3 Feb 2015 at 18:27:54, Sergiu Dumitriu ([email protected](mailto:[email protected])) wrote:

+1 for the idea. Some Qs about the implementation.

Do we have to add the @Rule in every test class?

Only in test classes where we need to capture logs.  I guess we could improve this by using the RunListener concept to do that so that it’s done automatically for all unit tests automatically. And at test end, if there are not asserted captured logs, then we display them in stdout so that the build can display it.

If yes, how do we ensure we don't forget to add it? I guess that's what CaptureConsoleRunListener does, right?

Yes. This RunListener captures stdout/stderr and fails if there’s content in either of them.

What about ancient tests that still use jmock and the old junit?

We need to move them to the new tools. In the meantime we can exclude them from the automated test. Thanks -Vincent

On 02/02/2015 04:45 AM, [email protected] wrote:

...

Hi devs,

I don’t recall if we had already agreed to this proposal or not so now that I’ve progressed with its implementation I’m sending this mail to ensure we all agree.

Rationale:

* The build logs should not be polluted with “normal” stack traces, or any message. Anything that the test produces should be contained and asserted in the test itself. * If some logs go through it means they’ve not been captured and not been asserted in the test. * It makes the test writer more aware of what the code being tested outputs and thus what the test writer needs to do about these.

Implementation details:

* Fail the build if some unit test output content to stdout or stderr. * By default any code that outputs log will send content to stdout (since by default slf4j/logback will use a Console Appender that logs to stdout). * Thus when code output logs, the test need to capture those logs. I’m introduced a new rule (@AllLogRule - see https://gist.github.com/vmassol/d6357901fca25db74783) which captures all logs. * The @AllLogRule rule will fail the test if the captured logs are not asserted (it can be asserted for its content or the test can explicitly say it doesn’t care about it). * There will be a custom RunListener (CaptureConsoleRunListener - see https://gist.github.com/vmassol/f3b7496b5bd3a9693fc2) plugged into the Maven Surefire plugin that fails the build if there’s content sent to stdut/stderr * Our Maven build will perform the check by default and we’ll start by disabling it in the modules that currently fail. * The idea is that slowly over time we remove the disabling in those modules as we fix the tests and that for new modules we apply the new rule (which will be enabled by default)

Example of capturing/asserting logs:

@Rule public AllLogRule logRule = new AllLogRule(); … assertEquals("Error getting resource [bad resource] because of invalid path format. Reason: [invalid url]", this.logRule.getMessage(0));

Example of ignoring a log:

… assertNotNull(this.logRule.getMessage(0));

WDTY?

Thanks -Vincent

-- Sergiu Dumitriu