arquillian / arquillian-governor

Arquillian Governor
Apache License 2.0
10 stars 11 forks source link

Governor Redmine #16

Closed rmpestano closed 8 years ago

rmpestano commented 8 years ago

Created redmine sub project.

It Adds the ability to manage Redmine issues through Arquillian governor.

smiklosovic commented 8 years ago

Yay! Awesome job! I'll look at this today :)

smiklosovic commented 8 years ago

Thanks for this PR a lot! I do not mind to merge it once some cosmetic changes are done to the code.

Please use this code formatter in your favorite IDE to make sure code is consistent accross the project. The only change to that coding style is that braces are put on new line. It can be easilly overridden in the IDE settings.

https://github.com/jboss/ide-config

rmpestano commented 8 years ago

Hi @smiklosovic, thanks for reviewing it.

There is something where I need some help here.

I wanted to add the ability to re open issues to governor redmine when a test related to a closed issue fails.

The problem is that when a test fails I receive testResult status == PASSED in execution decided here.

Do you have any idea why this is happening?

smiklosovic commented 8 years ago

@rmpestano

Honestly I do not know. That is something what @aslakknutsen could elaborate on. I was always thinking that when you recieve "After" event, the test result is fully populated / initialized to reflect it.

I think that we have seen this some time ago.

Please try to update Arquillian to 1.1.10.Final but that should not change a thing.

smiklosovic commented 8 years ago

@rmpestano try to listen to this https://github.com/arquillian/arquillian-core/blob/master/test/spi/src/main/java/org/jboss/arquillian/test/spi/event/suite/AfterTestLifecycleEvent.java

rmpestano commented 8 years ago

@smiklosovic I'll take a look but I remember the problem with 'false' test result was in 1.1.4.

About the code conventions and the brackets in new line I could not meke it work in IDEA (14.1.5) neither in eclipse (mars). In IDEA there is no 'In IDEA go to File -> Settings -> Import Settings' menu and in Eclipse the template has no effect. I will take a look at home later. Thank you so far.

rmpestano commented 8 years ago

Listening to AfterTestLifecycleEvent also didnt helped:

after_test_lifecycle

lets see what Aslak has to say

rmpestano commented 8 years ago

well it is just working, the only strange thing is that the AfterTestLifecycleEvent is called two times, one with testResult Status.PASSED and another with FAILED, in the end it behaved as I expected and the issue was reopened:

openclosed

smiklosovic commented 8 years ago

awesome! could you investigate if this is also the case in jira and github implementations and eventually repair it there? please create another branch snd pr for that. i will return to this in the evening hopefully and we can cut new version in a day or two

smiklosovic commented 8 years ago

from your first screenshot i see the is missing logger implementation related to slf4j, could you add some to class path if possible so it is logged properly, please?

aslakknutsen commented 8 years ago

You get double calls because this event is extended by After and AfterRule events. We don't know the final stage until the rules are ran, if there are any, because they execute outside the scope of our normal control..

So general rules is, there might be two events and if there is the last one is correct. :)

On Tue, Dec 22, 2015, 01:26 Rafael M. Pestano notifications@github.com wrote:

well it is just working, the only strange thing is that the AfterTestLifecycleEvent is called two times, one with testResult Status.PASSED and another with FAILED, in the end it behaved as I expected and the issue was reopened:

[image: openclosed] https://cloud.githubusercontent.com/assets/1592273/11944953/9ac6c482-a831-11e5-9ccf-2f052af2979d.png

— Reply to this email directly or view it on GitHub https://github.com/arquillian/arquillian-governor/pull/16#issuecomment-166465704 .

rmpestano commented 8 years ago

Thanks for the explanation @aslakknutsen.

I had to introduce another decision reason, one specific for opening issues for failed tests. see here.

Without this the execution enters in the the two 'ifs' for the same test. The first time when the test is with status passed and the second one with correct status.

WDYT?

smiklosovic commented 8 years ago

@rmpestano yeah we are doing something very similar here (1). You basically count the number of "afters" and in your case, the "if" will be executed only when you have the second event fired which will be reflected in that counter.

i guess your question was about this :)

(1) https://github.com/arquillian/arquillian-recorder/blob/master/arquillian-recorder-reporter/arquillian-recorder-reporter-impl/src/main/java/org/arquillian/recorder/reporter/impl/ReporterLifecycleObserver.java#L161-L211

rmpestano commented 8 years ago

@smiklosovic, I see. I am also checking if the issue isn't closed (or opened) to decide if I update it.

An issue I'm facing now is related to Redmine itself. We can have workflows for issue statuses transition so for example here at the company we can't close an issue directly, we have first to set its status to 'execution' and then we are able to close.

The problem is that redmine rest api doesn't provide information about workflow, see here.

I am thinking in provide a property at arquillian.xml so user can define the close workflow, somethink like:

<extension qualifier="governor-redmine">
        <property name="closeStatusWorkflow">2,5</property>
    </extension>

this means that issue will be updated to status 2(execution) and 5(closed). Its not ideal cause developer must know the status ids (it can be found in the redmineURL/issue_statuses.xml).

I think Jira has something similar but as I see it has an api for issue transitions.

What do you think?

smiklosovic commented 8 years ago

@rmpestano could not you parse that issue_statuses.xml directly to know the status so developer himself does not have to do that?

rmpestano commented 8 years ago

I can (it is in the REST api) but I'm not sure how it could help cause the main problem is the status update order. In the above example it was 2, 5 but it could be 2, 4, 8 , 5 and so on. It depends on how redmine was configured. Só somehow developer will have to provide that status transition order (he will know cause it is the sane status order he updates issues through redmine web application).

rmpestano commented 8 years ago

Or you mean just to print in the logs all available status and its ids at test startup?

smiklosovic commented 8 years ago

i dont know ... do it in the way it is the most comfortable and viable for you. I will merge it anyway

rmpestano commented 8 years ago

Hi again,

The only problem now is the code style, I still cannot apply jboss formater here, look my eclipse(developer studio 8.1.0) config:

formatter

rmpestano commented 8 years ago

Hi @smiklosovic,

just confirmed that github governor also suffers from the multiple event calling, see #17.

rmpestano commented 8 years ago

I have commited the fix for github governor and it is incorporated in this PR. Don't know if it should be in this PR.

smiklosovic commented 8 years ago

thanks! i will return to this on 27th of december, merry christmas!

smiklosovic commented 8 years ago

what about jira?

smiklosovic commented 8 years ago

dont worry about codestyle, i will format it before squash and merge

rmpestano commented 8 years ago

yea same for jira, applyed the same fix.

merry christmas!

rmpestano commented 8 years ago

Hi @smiklosovic,

now I think we are ready.

Besides adding governor Redmine, this PR solves #19 and #20.

19 affected only github and #20 afected both github and jira.

smiklosovic commented 8 years ago

I cherry-picked your last commits and squashed them on top of already-merged master

Thanks! You can expect release soon.

rmpestano commented 8 years ago

:+1: