[ARQ-606] Fixed failing deployment upon GlassFish WARNING #4

Merged
merged 1 commit into from Oct 4, 2011

Projects

None yet

3 participants

@PiotrNowicki

Fixing test failure because of GlassFish persistence warnings [1].
Switched to recognize not only non-success status of Glassfish, but also warnings, so:

  1. No exit code - Exception,
  2. Warning exit code - log.warning(-) but move along,
  3. Non-success code - Exception

Hope it's acceptable.

PS. As in previous commits - I see a lot of deletions/insertions in commit diff. I guess this is also because I am using the JBoss code formatter.

[1] - https://issues.jboss.org/browse/ARQ-606

@zpaulovics

Hi,

Good addition! I support your pull request.

Please, review your log message.

Below is an example from your log output:

WARNING: Deployment resulted in a warning: exit_code: WARNING, message: "http://localhost:4848/management/domain/applications/application/test" created successfully.

PER01003: Deployment encountered SQL Exceptions:
PER01000: Got SQLException executing statement "CREATE TABLE EJB3_1_SEQUENCE_GENERATOR (GEN_KEY VARCHAR2(50) NOT NULL, GEN_VALUE NUMBER(38) NULL, PRIMARY KEY (GEN_KEY))": java.sql.SQLException: ORA-00955: name is already used by an existing object

Consider the above arrangement.

WARNING: message: "http://localhost:4848/management/domain/applications/application/test" created successfully.

PER01003: Deployment encountered SQL Exceptions:
PER01000: Got SQLException executing statement "CREATE TABLE EJB3_1_SEQUENCE_GENERATOR (GEN_KEY VARCHAR2(50) NOT NULL, GEN_VALUE NUMBER(38) NULL, PRIMARY KEY (GEN_KEY))": java.sql.SQLException: ORA-00955: name is already used by an existing object

Regards,

Zoltan

@PiotrNowicki

Hi Zoltan!

So you suggest to remove the "Deployment resulted in a warning: exit_code: WARNING,"?

Ok, so the log level says that it's the warning, but I still think it might be relevant to left any kind of information about the Glassfish response exit-code, i.e. for times when we change the log level to 'info' or 'fine'. Basing on logs we wouldn't have any information that the Glassfish deployment resulted in a warning.

What do you think?
Cheers!

@zpaulovics

Hi Pedro!

I have spent some exercises using your pull request during the weekend. At first look it seemed the "Deployment resulted in a warning: exit_code: WARNING," is not needed, because it has too many "WARNINGs" in the message:-). But having same different cases of deployment with "WARNING" messages, I have realized that it is better to leave your error message as it is.

We face two aspects here:

  1. Inform the developer about the status of the test deployment. It is typically successful when we got a "warning". The message has this information. So it is O.K.
  2. Whether or not we have any real issues on GlassFish level?
    a. Several times the message from GlassFish is not a real problem (mainly related with persistence), but the information is important to see what is going on. That is your pull request all about. This kind of situation the test can be completed successfully.
    b. Some times the message from GlassFish is a real problem that fails our test. But providing the available information about the issue's reason, helps a lot in fixing the problem.

So, after playing some time with your improved solution, a realized that it is better to leave your massage as it is, because in different cases we need different peace of the information from the message.

Cheers,

Zoltan

@PiotrNowicki

Thanks for your opinion Zoltan!

Cheers

@PiotrNowicki

Ok, I've reformatted the commit, so I think it's much more readable right now.

However, I must suggest to push one commit which only reformats the code in one style.
I was unable to edit the file in Eclipse, because without touching anything the formatter was applied. Even when the one posted by Aslak (http://pastebin.com/xnBxYZZk) was used, simply opening the file generated ca. 200 changes in the code.

Had to do the changes in the VIM :-)

Cheers!

PS. Zoltan, I've thought about your warnings/logging remarks and maybe it would be better to change the logs to more clearly inform the developer about the REST status and glassfish status? Despite the fact of too many 'warnings' in the log, it looks somewhat unclear (warning, warning, warning, successful - at least the finish is positive ;-) :
"WARNING: Deployment resulted in a warning: exit_code: WARNING, message: "http://localhost:4848/management/domain/applications/application/test" created successfully."

WDYT?

@aslakknutsen aslakknutsen merged commit 9063abf into arquillian:master Oct 4, 2011
@aslakknutsen
Member

Excellent! Pushed upstream

We'll do a complete reformat of all code after 1.0.0.Final release. :)

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