You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by jbertram <gi...@git.apache.org> on 2018/02/20 15:16:53 UTC

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

GitHub user jbertram opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1881

    ARTEMIS-1684 fail build for failed 'fast' tests

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-1684

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/1881.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1881
    
----
commit c19914a2058420166457d8d5131c3eca9df432d0
Author: Justin Bertram <jb...@...>
Date:   2018-02-20T15:16:09Z

    ARTEMIS-1684 fail build for failed 'fast' tests

----


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169482105
  
    --- Diff: .travis.yml ---
    @@ -1,7 +1,7 @@
     sudo: false
     language: java
     install: true
    -script: mvn -Pfast-tests -Pextra-tests -B install
    +script: mvn -Pfast-tests -Pextra-tests -B test
    --- End diff --
    
    I told you in private.  That is to make sure the extra tests will compile. 


---

[GitHub] activemq-artemis issue #1881: ARTEMIS-1684 fail build for failed 'fast' test...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1881
  
    @jbertram did you look into this failure?


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1881


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169705458
  
    --- Diff: .travis.yml ---
    @@ -1,7 +1,7 @@
     sudo: false
     language: java
     install: true
    -script: mvn -Pfast-tests -Pextra-tests -B install
    --- End diff --
    
    Perhaps you could add a comment (don't know what's the format for comments on yml) documenting why the install is needed?


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169452802
  
    --- Diff: .travis.yml ---
    @@ -1,7 +1,7 @@
     sudo: false
     language: java
     install: true
    -script: mvn -Pfast-tests -Pextra-tests -B install
    +script: mvn -Pfast-tests -Pextra-tests -B test
    --- End diff --
    
    I'm confused about this in regard to the previous commit. Did you mean -Pextra-tests


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169374458
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/NetworkHealthTest.java ---
    @@ -159,7 +159,7 @@ public void testParseLogger() throws Exception {
           Assert.assertEquals(2, check.getUrls().size());
        }
     
    -   @Test
    +   //@Test
    --- End diff --
    
    Will do.


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169705162
  
    --- Diff: .travis.yml ---
    @@ -1,7 +1,7 @@
     sudo: false
     language: java
     install: true
    -script: mvn -Pfast-tests -Pextra-tests -B install
    --- End diff --
    
    smoke-tests are depending on the install, as they will create the broker from the artemis-home under the distribution
    if you remove this, those tests will not build.. as the build is failing for you now.


---

[GitHub] activemq-artemis issue #1881: ARTEMIS-1684 fail build for failed 'fast' test...

Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1881
  
    I'm looking into it now.  Trying to reproduce the failure locally, but have been unsuccessful so far.


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169664598
  
    --- Diff: .travis.yml ---
    @@ -1,7 +1,7 @@
     sudo: false
     language: java
     install: true
    -script: mvn -Pfast-tests -Pextra-tests -B install
    +script: mvn -Pfast-tests -Pextra-tests -B test
    --- End diff --
    
    I pulled that commit out so -Pextra-tests will remain in place.


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169429443
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/NetworkHealthTest.java ---
    @@ -179,6 +182,7 @@ private void doCheck(String localaddress) throws Exception {
     
        }
     
    +   @Ignore("doesn't work in Travis CI environment")
    --- End diff --
    
    From what I can tell the failure is due to the way the ping command is being constructed and executed in org.apache.activemq.artemis.core.server.NetworkHealthCheck#purePing (i.e. via java.lang.ProcessBuilder). It appears the Travis CI environment simply won't allow it.  I'll investigate creating an assumption to test the environment.


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169453963
  
    --- Diff: .travis.yml ---
    @@ -1,7 +1,7 @@
     sudo: false
     language: java
     install: true
    -script: mvn -Pfast-tests -Pextra-tests -B install
    +script: mvn -Pfast-tests -Pextra-tests -B test
    --- End diff --
    
    The two commits are independent.  One is to change 'install' to 'test'.  The other is to remove '-Pextra-tests' as it's not actually doing anything in the build as far as I can tell.


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169373030
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/NetworkHealthTest.java ---
    @@ -159,7 +159,7 @@ public void testParseLogger() throws Exception {
           Assert.assertEquals(2, check.getUrls().size());
        }
     
    -   @Test
    +   //@Test
    --- End diff --
    
    can you use @Ignore instead.
    
    At least the test would be runnable within the IDE.


---

[GitHub] activemq-artemis pull request #1881: ARTEMIS-1684 fail build for failed 'fas...

Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1881#discussion_r169394169
  
    --- Diff: artemis-commons/src/test/java/org/apache/activemq/artemis/utils/NetworkHealthTest.java ---
    @@ -179,6 +182,7 @@ private void doCheck(String localaddress) throws Exception {
     
        }
     
    +   @Ignore("doesn't work in Travis CI environment")
    --- End diff --
    
    Is this because its failing due to lack of IPV6 in the env? If the test really needs that it should probably check it works and fail an assumption which will make the test skip in envs it cant possibly work in, as opposed to disabling the test everywhere.


---