New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align ST Logger also with Maven logging format #133

Closed
MatousJobanek opened this Issue Sep 8, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@MatousJobanek
Contributor

MatousJobanek commented Sep 8, 2017

In ST we use our own Logger that reflects the way of logging used in Surefire providers. In the original surefire providers, they are using their own implementations (based on the information if the execution is using a fork or not). But in any case, they log on stdout. This is also why I have implemented it using stdout and stderr.

The current state is fine in case of logging in the phase of provider execution - before/during/after test invocation. However, there is also Maven extension that logs much earlier and should reflect the way of logging that is used by Maven. For this, Maven provides injection of org.codehaus.plexus.logging.Logger.

Our Logger should reflect the phase when it is used and should use the correct way of logging.

@hemanik

This comment has been minimized.

Show comment
Hide comment
@hemanik

hemanik Sep 18, 2017

Contributor

Logging in the maven extension phase, either using plexus maven logger or smart testing logger logs to the stdout.

Different approaches to align ST Logger with Maven Logging as proposed by @MatousJobanek are:

  1. Just alternate the prefix INFO to [INFO] when we are in the maven extension phase - to keep to formatting that is used by maven.
    [This approach has already been taken care of by @bartoszmajsak earlier commits.]

  2. Integration with plexus logger provided as DI.

Drawbacks of using the second approach in my opinion are:

  • we introduce an overhead of using an additional logger, with no real benefits that contradicts with our initial thoughts on unifying the logging experience.
  • the logging included in the maven extension phase of our project is more related to our smart-testing-extension rather than to maven in specific. (So it makes sense to use our logger with the prefix [INFO] [Smart Testing Extension] )
  • another issue is that the plexus logger checks for maven debug log level before logging, so if -X flag is not set by the user, the logging done using plexus logger will not be displayed if only smart.testing.debug property is set.

@MatousJobanek , @bartoszmajsak , @lordofthejars , @dipak-pawar, your thoughts ?

Contributor

hemanik commented Sep 18, 2017

Logging in the maven extension phase, either using plexus maven logger or smart testing logger logs to the stdout.

Different approaches to align ST Logger with Maven Logging as proposed by @MatousJobanek are:

  1. Just alternate the prefix INFO to [INFO] when we are in the maven extension phase - to keep to formatting that is used by maven.
    [This approach has already been taken care of by @bartoszmajsak earlier commits.]

  2. Integration with plexus logger provided as DI.

Drawbacks of using the second approach in my opinion are:

  • we introduce an overhead of using an additional logger, with no real benefits that contradicts with our initial thoughts on unifying the logging experience.
  • the logging included in the maven extension phase of our project is more related to our smart-testing-extension rather than to maven in specific. (So it makes sense to use our logger with the prefix [INFO] [Smart Testing Extension] )
  • another issue is that the plexus logger checks for maven debug log level before logging, so if -X flag is not set by the user, the logging done using plexus logger will not be displayed if only smart.testing.debug property is set.

@MatousJobanek , @bartoszmajsak , @lordofthejars , @dipak-pawar, your thoughts ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment