arquillian / arquillian-governor

Arquillian Governor
Apache License 2.0
10 stars 11 forks source link

Add support for multiple servers in JIRA Governor Extension #22

Open basovnik opened 8 years ago

basovnik commented 8 years ago

It would be great to support several servers. It is useful when you have different jira servers for community and product. But there can be conflict in names. So @Jira annotation could have optional argument to define server id.

smiklosovic commented 8 years ago

Implemening that would not be a trivial task. In order to contact some Jira server, you have to provide credentials in arquillian.xml. So you would have to abstract whole extension to deal with multiple Jira servers from the configuration point of view and you would need to put them in some registry and dynamically switch between them.

smiklosovic commented 8 years ago

I have looked into the code and infrastructure-wise the code is ready. There is good abstraction of JiraGovernorClient and you make them by JiraGovernorClientFactory which produces client from the passed JiraGovernorConfiguration.

The only thing is to implement parsing of multiple Jira servers in configration from the arquillian.xml. From that point on, it is only the implementation task to put them to some registry and resolve between them in execution decider.

If you figure out how to configure multiple Jira servers in arquillian.xml, I will do the rest. Try to send some kind of proposal how you would do that, even theoretically.

smiklosovic commented 8 years ago

My proposal is this:

<arquillian>
    <extension qualifier="governor-jira">
        // properties for the first jira server
    </extension>
    <extension qualifier="governor-jira-server2">
        // properties for the second jira server
    </extension>
</arquillian>

So you would have to do it like this:

// this would automatically use second extension in above
@Jira(server = "server2")

// this would use the first
@Jira

If you use it like this:

<arquillian>
    <extension qualifier="governor-jira-server1">
        // properties for first jira server
    </extension>
    <extension qualifier="governor-jira-server2">
        // properties for the second jira server
    </extension>
</arquillian>
@Jira // this would fail

// this would pass
@Jira(server = "server1")
@Jira(server = "server2")

What do you think?

smiklosovic commented 8 years ago

Maybe it could be done with annotations as well.

The same way you put @Jira on a test method, you would put server annotation on it.

@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.ANNOTATION_TYPE})
@Documented
public @interface Server
{
}

and after that:

@Server
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.METHOD })
@Documented
public @interface Community
{
}

and another one:

@Server
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.METHOD })
@Documented
public @interface Production
{
}

You would have to have extensions with names "governor-jira-community" and "governor-jira-production". In other words, lowercased server annotation would equal to suffix in respective extension qualifier in arquillian.xml. The same approach is used in Arquillian Drone with Drone qualifiers.

So you would do:

@Test
@Jira
@Community
public void testMethod()
{
}

@Test
@Jira
@Production
public void testMethod2()
{
}

The problem is what server would be contacted if you do not put any @Server annotation on it. In that case, it would expect the configuration in extension with qualifier "governor-jira" (hence it does not have any suffix).

I do not know if this is reasonably implementable yet but it does make sense to me.

basovnik commented 8 years ago

My next feature request will be to support defining of multiple issues for one test.

Possible solutions: A)

@Jira({"A-1","B-1"})

B)

@Jiras({
  @Jira("A-1"),
  @Jira("B-1")
})

How to define server definition here? Special annotation will not work if component "A" and "B" are on different servers. So I would prefer the solution B with optional parameter, e.g. server.

Possible solution:

@Jiras({
  @Jira(id="A-1", server="community"),
  @Jira(id="B-1", server="production")
})

The most generic solution I would like :-)

@Issues({
  @Jira(id="A-1", server="jira-community"),
  @Jira(id="B-1", server="jira-production"),
  @Bugzilla(id="C-1"),
  @Github(id="D-1"),
  @Gitlab(id="E-1")
})

If there are more possible servers I would check that server parameter must be set.

Extension definition could look like this:

<extension qualifier="governor-issues">
      <property name="servers">
          jira-community:
            type: jira
            server: https://issues.apache.org/jira
          jira-production:
            type: jira
            server: https://issues.jboss.org
          bugzilla:
            type: bugzilla
            server: http://bugzilla.redhat.com
          github:
            type: github
            server: https://github.com/arquillian/arquillian-governor
          gitlab:
            type: gitlab
            server: ????
      </property>
  </extension>

(I was inspired with dockerContainers property in arquillian-cube).

What do you think?

smiklosovic commented 8 years ago

While technically doable I am not fan of it. I think you make it just too complicated.

Firstly, my "extension per server" approach is more clean then putting all stuff into one extension in some yaml and it is just better readable and maintainable.

Secondly, from Java 8 on, you can use multiple annotations of the same type (not only) on a method so you could write this:

@Test
@Jira("ARQ-1")
@Jira("ARQ-2")
@Production
public void test() {}

Thridly, I do not quite believe that there is a use case for having two Jiras on different servers which are dealing with one test method, however, let's pretend there is.

My point of using @Community and @Production annotations would make it more easy to write these test methods because I would personally find writing server=community everywhere quite tedious and mundane task. It is easier to add an annotation to it.

However, keeping in mind your request - I would then implement it as you want - by adding server option to @Jira annotation as you have suggest.

In case you have to stick with Java 6 / 7 at work and not execute it with Java 8, multiple annotations of the same type on a test method are out of question so I would have to wrap it into @Jiras annotation but that would make it harder and it would break backward compatibility of API. Or at least I would just have to maintain both options which I am not tending to.

Is that really want you want?

smiklosovic commented 8 years ago

As far as I remember, you can use multiple Governor annotations of different type on a test method right now out of the box ...

basovnik commented 8 years ago

ad Firstly) I think that it is actually only one extension with more complex configuration. ad Secondly) Java 7 is supported JDK with our product and we need it. ad Thirdly) I definitely have use-case with testing of Apache Camel.

I agree that my proposal is too complex. But our team would really use all the issue sources which I mentioned (Jira, BZ, Github, Gitlab).

We are willing to contribute to this project and implement this feature but firstly we have to agree on something.

smiklosovic commented 8 years ago

Ok. So we do it like this:

we will go with my proposal of extension per server and there will be "server= " arguments in @Jira annotation.

<arquillian>
    <extension qualifier="governor-jira">
        // properties for first jira server
    </extension>
    <extension qualifier="governor-jira-server1">
        // properties for second jira server
    </extension>
    <extension qualifier="governor-jira-server2">
        // properties for the third jira server
    </extension>
</arquillian>

There will be @Jiras annotation which will wrap @Jira annotations which will have optional "server" argument on it. server argument would have value equal to a suffix of some extension.

Example:

@Test
@Jiras({
    @Jira("X"),
    @Jira(id = "Y", server = "server1"),
    @Jira(id = "Z", server = "server2")
})
public void test() {}

In case there is just one Jira server, it would look like this:

@Test
@Jiras(@Jira("X"))

Theoretically like this:

@Test
@Jiras(@Jira("X", server="server1"))

I am not sure I am ok with it. I do not like the fact you have to wrap it when you have just one Jira issue. I try to support the original version as well but I am not sure how difficult that will be.

1) It will be illegal to put multiple @Jiras annotations on a test method. 2) In case of one Governor extension without suffix, server field will be empty 3) There will not be used any Java 8 specific language feature so we will be Java 7 compatible as you prefer.

Lastly, there is not BZ integration yet and I would love to merge it. We have GitHub, Jira and Redmine covered. I do not know how GitLab is different from GitHub. I think they differ but I don't know how much.

Currently, @Jira annotation have force field which says if you want to execute a test method even it should not normally. How do you want to resolve a situation when one @Jira have it true and other false?

There are test execution deciders which decide if respective @Jira test should be executed or not. These strategies can produce different execution decisions in case you have two @Jira annotations on it which are resolving it against two different Jira instances. In other words, one server says to you that you should execute this test method. The second server says to you that you should not. What would you do?

basovnik commented 8 years ago
public @interface Jiras {
    Jira[] list();
    boolean force() default false;
}

What do you think?

smiklosovic commented 8 years ago

OK, I will look into it in a few days and let you know how I am doing.

MatousJobanek commented 8 years ago

Hi, just small insights: regarding arq.xml configuration. Stefan - what would you reference to from the java code when you would want to use the first server - there is no name. What about using combination of your two proposals:

<extension qualifier="governor-jira">
    <servers>
        <server name="production">
            <url>
            <password>
             ...
        </server>
        <server name="community">
            <url>
            <password>
             ...
        </server>
    <servers>
</extension>

If only one server is used, then it can be defined in old form (backward compatibility) or can be defined as one single server in a <servers> tag (open to next addition).

@Jiras{} +1 however, I would not wrap one single @Jira within the wrapper.

smiklosovic commented 8 years ago

default server name will be "default" (that will be the value in the Jira annotation) and in case I parse extension without suffix, I will register it under "default" name - so the current behaviour when you have just one server will be the same.

Yes I aggree with you on not wrapping single Jira in Jiras.

If you want to add <servers> into the xml, that would change xml structure and it would not be valid anymore. You can not get it from the ArquillianDescriptor as well.

MatousJobanek commented 8 years ago

the "default" name sounds good :-)

BTW. currently - is there possibility to have one test method annotated by two annotations @Jira and @Github ?

smiklosovic commented 8 years ago

yes it should work

MatousJobanek commented 8 years ago

Thanks. I was asking because I wasn't sure whether the @Issues would be useful. And you confirmed that it wouldn't :-).

basovnik commented 8 years ago

Stefan, are you going to implement it? I can try it if you do not have time.

smiklosovic commented 8 years ago

i will try to do it during weekend, after that, it is your turn

smiklosovic commented 8 years ago

tests are not written properly, what do you think?

https://github.com/arquillian/arquillian-governor/commit/62cd8df2b903bbb5a33efc76daac1936537031f5

smiklosovic commented 8 years ago

@basovnik i put there another commit but I am not sure if it is good path.

the thing is that I do not know what to do when you have Jiras case. You said that if you put force on the execution, if it is true, then "force" in "Jira" annotation will be skipped.

But if force is true and that method will be executed (or it is not true but you have just multiple jiras in that array) if you are about to close that issue, how do you know against which jira server (wrapped Jira annotation) it was resolved?

basovnik commented 8 years ago

@smiklosovic, I am not sure if I understand you. But when you invoke a test case you catch AfterTestLifecycleEvent with Jiras annotation definition containing all required metadata including server property. Then you can close every Jira with property closePassed=true. We can assume the if test passed then all issues are fixed. Could you try to describe me your problem once more?

basovnik commented 8 years ago

@smiklosovic Hi Stefan, are you working on this issue now? If not, I would like to have new release containing last few commits.

smiklosovic commented 8 years ago

Sure, do the job and I will gladly merge!

basovnik commented 8 years ago

I thought that you will finish this issue. Our necessary commits with detectors support are already in release 1.0.3.Final. This issue should be in the next release. Are you going to finish this issue? You have already spent some time with this issue...

smiklosovic commented 8 years ago

just do it please

bartoszmajsak commented 8 years ago

@basovnik @smiklosovic What is the status of it? We can consider taking it over if there is some initial work done.