Cactus
  1. Cactus
  2. CACTUS-86

jboss port is not changed if cactus.port is used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.7.1
    • Component/s: Maven Integration
    • Labels:
      None
    • Environment:
      Operating System: Other
      Platform: All

      Description

      The cactus plugin offers a property named cactus.port. This property contains
      the port to which it should connect to start the tests.

      Chaging the cactus.port to 8880 does not change the URL being invoked by the plugin.

      I can provide a patch that solves this issue, provided you give me a path to the
      sources I have to patch.

        Issue Links

          Activity

          Hide
          Brian Topping added a comment -

          This should be much easier once the patch that recently submitted is
          applied... or is it applied and I broke something?

          Show
          Brian Topping added a comment - This should be much easier once the patch that recently submitted is applied... or is it applied and I broke something?
          Hide
          Vincent Massol added a comment -

          Hi Stephane,

          Yes, you're right. This property is used by all containers except for JBoss.
          The reason is simply because we are not supporting it yet for JBoss. If you
          can supply a patch, I'll gladly apply it.

          Thanks
          -Vincent

          PS for Brian: Your patch was for the root context but not for the port, right?

          Show
          Vincent Massol added a comment - Hi Stephane, Yes, you're right. This property is used by all containers except for JBoss. The reason is simply because we are not supporting it yet for JBoss. If you can supply a patch, I'll gladly apply it. Thanks -Vincent PS for Brian: Your patch was for the root context but not for the port, right?
          Hide
          Brian Topping added a comment -

          Yes, that's right. But adding this functionality should be much easier now
          that things have been refactored for it at the container level.

          Show
          Brian Topping added a comment - Yes, that's right. But adding this functionality should be much easier now that things have been refactored for it at the container level.
          Hide
          Stephane Nicoll added a comment -

          Created an attachment (id=9905)
          Jboss port patch (HEAD)

          Show
          Stephane Nicoll added a comment - Created an attachment (id=9905) Jboss port patch (HEAD)
          Hide
          Stephane Nicoll added a comment -

          The patch does not remove the TODO line

          <!-- TODO: Find how to set the port for JBoss 3x -->

          Let's first validate it works

          Regards,

          Stephane

          PS: maybe a stupid question, but how the hell can I jar the maven plugin from CVS?

          Show
          Stephane Nicoll added a comment - The patch does not remove the TODO line <!-- TODO: Find how to set the port for JBoss 3x --> Let's first validate it works Regards, Stephane PS: maybe a stupid question, but how the hell can I jar the maven plugin from CVS?
          Hide
          Vincent Massol added a comment -

          Hi Stephane,

          Thanks for the patch. Have you tried it? I don't see how it can work as there
          is no implementation of what to do with the port in the JBoss container
          class... (to my knowledge). The prepare() method is the one that needs to be
          patched (from the top of my head). You can check the other containers to see
          how it works: there is a @cactus.port@ filter token in their config files
          which is replaced at runtime by the value.

          In order for this to work you probably also need to patch the way we deploy in
          JBoss. ATM, we reuse an existing JBoss server config (server/default) but we
          need to do it the way we do it for other containers, i.e. create a separate
          config for our tests. Again you can check the implementation for the other
          containers to get more insight into this!

          Thanks for your help!
          -Vincent

          Show
          Vincent Massol added a comment - Hi Stephane, Thanks for the patch. Have you tried it? I don't see how it can work as there is no implementation of what to do with the port in the JBoss container class... (to my knowledge). The prepare() method is the one that needs to be patched (from the top of my head). You can check the other containers to see how it works: there is a @cactus.port@ filter token in their config files which is replaced at runtime by the value. In order for this to work you probably also need to patch the way we deploy in JBoss. ATM, we reuse an existing JBoss server config (server/default) but we need to do it the way we do it for other containers, i.e. create a separate config for our tests. Again you can check the implementation for the other containers to get more insight into this! Thanks for your help! -Vincent
          Hide
          Stephane Nicoll added a comment -

          Vincent,

          Of course I tested before. Actually, It was so simple I found it souspicious.
          Well, just to be clear, I used:

          Maven 1.0RC1
          Jboss plugin 1.3
          Cactus plugin as it is now.

          I put the following properties in my maven project:

          #Cactus
          cactus.home.jboss3x=/var/jboss
          cactus.jboss3x.config.name=test
          cactus.port=8880

          I created a test server on /var/jboss/server which is a copy of default (the
          diffrence being my user owns it so that I can start jboss propertly)

          Without the patch it does not work. With the patch cactus goes indeed on the
          right port.

          Regards,

          Stephane

          Show
          Stephane Nicoll added a comment - Vincent, Of course I tested before. Actually, It was so simple I found it souspicious. Well, just to be clear, I used: Maven 1.0RC1 Jboss plugin 1.3 Cactus plugin as it is now. I put the following properties in my maven project: #Cactus cactus.home.jboss3x=/var/jboss cactus.jboss3x.config.name=test cactus.port=8880 I created a test server on /var/jboss/server which is a copy of default (the diffrence being my user owns it so that I can start jboss propertly) Without the patch it does not work. With the patch cactus goes indeed on the right port. Regards, Stephane
          Hide
          Vincent Massol added a comment -

          Ah ok, I understand now. You're actually doing a manual part, which is the
          creation of the JBoss test server config. We have automated this for all
          containers except JBoss (just because of a lack of time).

          What I do not like with this patch is that it'll be misleading because the
          default JBoss config is configured to use port 8080. Thus users will put
          cactus.port = whatever and it'll not work! They'll then report it as a bug...

          This the reason we haven't yet applied this patch (it was also suggested by
          someone else some time ago). What we could do is test if cactus.port is !=
          than 8080 and display a warning in plugin.jelly (for JBoss only), telling the
          user that either must edit the default config file or use a custom config.
          Alternatively, we could only make this port work when a custom server config
          is used.

          Of course, the best solution is to support an automated Cactus test server
          config as we're doing for the other containers.

          Thanks
          -Vincent

          Show
          Vincent Massol added a comment - Ah ok, I understand now. You're actually doing a manual part, which is the creation of the JBoss test server config. We have automated this for all containers except JBoss (just because of a lack of time). What I do not like with this patch is that it'll be misleading because the default JBoss config is configured to use port 8080. Thus users will put cactus.port = whatever and it'll not work! They'll then report it as a bug... This the reason we haven't yet applied this patch (it was also suggested by someone else some time ago). What we could do is test if cactus.port is != than 8080 and display a warning in plugin.jelly (for JBoss only), telling the user that either must edit the default config file or use a custom config. Alternatively, we could only make this port work when a custom server config is used. Of course, the best solution is to support an automated Cactus test server config as we're doing for the other containers. Thanks -Vincent
          Hide
          Stephane Nicoll added a comment -

          Vincent,

          I didn't knew about this container creation stuff ...

          >What I do not like with this patch is that it'll be misleading because the
          >default JBoss config is configured to use port 8080. Thus users will put
          >cactus.port = whatever and it'll not work! They'll then report it as a bug...

          What's the purpose of the cactus.port then? Whether you create the whole test
          container or not, users may want it not to run on 8080 (because they have
          already something else, like a tomcat running on their local copy). If you
          provide this option, then either you let the user create its jboss container
          itself, either you make sure that the container that will be automtically
          created by the plugin uses the port specified by the user.

          There's not much stuff to do to change the HTTP port (a simple grep -ri 8080 on
          the deploy directory will give you two xml to change).

          Also, this container creation seems weird to me. In our case, we have a lot of
          custom libs and standard jar and war applications deployed in our test
          container. If you plan to create it automatically, are you providing a way to
          customize the created container somehow?

          Regards,

          Stephane

          Show
          Stephane Nicoll added a comment - Vincent, I didn't knew about this container creation stuff ... >What I do not like with this patch is that it'll be misleading because the >default JBoss config is configured to use port 8080. Thus users will put >cactus.port = whatever and it'll not work! They'll then report it as a bug... What's the purpose of the cactus.port then? Whether you create the whole test container or not, users may want it not to run on 8080 (because they have already something else, like a tomcat running on their local copy). If you provide this option, then either you let the user create its jboss container itself, either you make sure that the container that will be automtically created by the plugin uses the port specified by the user. There's not much stuff to do to change the HTTP port (a simple grep -ri 8080 on the deploy directory will give you two xml to change). Also, this container creation seems weird to me. In our case, we have a lot of custom libs and standard jar and war applications deployed in our test container. If you plan to create it automatically, are you providing a way to customize the created container somehow? Regards, Stephane
          Hide
          Vincent Massol added a comment -

          > What's the purpose of the cactus.port then?

          To change the port for all containers that support this feature (i.e. all of
          them except JBoss for now).

          > Also, this container creation seems weird to me. In our case, we have a lot
          > of custom libs and standard jar and war applications deployed in our test
          > container. If you plan to create it automatically, are you providing a way
          > to customize the created container somehow?

          It is a default container config. Whenever you do more complex stuff, you
          must use a custom config (this is the reason for the
          cactus.jboss3x.config.name property you're using).

          My point is that we cannot assume everyone will be using a custom config.

          Thanks
          -Vincent

          Show
          Vincent Massol added a comment - > What's the purpose of the cactus.port then? To change the port for all containers that support this feature (i.e. all of them except JBoss for now). > Also, this container creation seems weird to me. In our case, we have a lot > of custom libs and standard jar and war applications deployed in our test > container. If you plan to create it automatically, are you providing a way > to customize the created container somehow? It is a default container config. Whenever you do more complex stuff, you must use a custom config (this is the reason for the cactus.jboss3x.config.name property you're using). My point is that we cannot assume everyone will be using a custom config. Thanks -Vincent
          Hide
          Stephane Nicoll added a comment -

          Vincent,

          I understand. Can you provide links where I can document myself about this
          container creation. I can maybe help on this for Jboss.

          Regards,

          Stephane

          Show
          Stephane Nicoll added a comment - Vincent, I understand. Can you provide links where I can document myself about this container creation. I can maybe help on this for Jboss. Regards, Stephane
          Hide
          Vincent Massol added a comment -

          Stephane,

          I guess the best place to look for documentation is in the code itself. You
          can have a look at the following classes:

          • Resin2xContainer
          • Tomcat?xContainer

          They setup custom test configs for Resin and Tomcat.

          Thanks. Supporting JBoss in the same manner would be great!
          -Vincent

          Show
          Vincent Massol added a comment - Stephane, I guess the best place to look for documentation is in the code itself. You can have a look at the following classes: Resin2xContainer Tomcat?xContainer They setup custom test configs for Resin and Tomcat. Thanks. Supporting JBoss in the same manner would be great! -Vincent
          Hide
          Stephane Nicoll added a comment -

          I am coming back to you about this patch. I am wondering: "till we have a custom
          container deployment, why don't we apply this patch because anyway the user will
          have to setup her/his jboss properly".

          Also, I assume that cactus.port will be the one used when we'll create the Jboss
          custom container, so it seems to me it make sense to apply the patch.

          What do you think?

          Show
          Stephane Nicoll added a comment - I am coming back to you about this patch. I am wondering: "till we have a custom container deployment, why don't we apply this patch because anyway the user will have to setup her/his jboss properly". Also, I assume that cactus.port will be the one used when we'll create the Jboss custom container, so it seems to me it make sense to apply the patch. What do you think?
          Hide
          Vincent Massol added a comment -

          Applied patch. I'm still leaving this bug opened as it doesn't solve the real
          problem.

          Thanks.

          Show
          Vincent Massol added a comment - Applied patch. I'm still leaving this bug opened as it doesn't solve the real problem. Thanks.
          Hide
          Vincent Massol added a comment -

          Ability to change the JBoss port will be implemented in Cargo once it supports JBoss (very soon now hopefully).

          Show
          Vincent Massol added a comment - Ability to change the JBoss port will be implemented in Cargo once it supports JBoss (very soon now hopefully).
          Hide
          Felipe Leme added a comment -

          Vincent,

          I've been using these ports for quite a while; it took me some time to understand why we shouldn't fix the bug (i.e., because the user must also change JBoss files).

          So, I put some comments on the plugin.properties and am marking it as closed.

          – Felipe

          Show
          Felipe Leme added a comment - Vincent, I've been using these ports for quite a while; it took me some time to understand why we shouldn't fix the bug (i.e., because the user must also change JBoss files). So, I put some comments on the plugin.properties and am marking it as closed. – Felipe

            People

            • Assignee:
              Vincent Massol
              Reporter:
              Stephane Nicoll
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development