You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by gaohoward <gi...@git.apache.org> on 2015/05/29 09:46:55 UTC
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
GitHub user gaohoward opened a pull request:
https://github.com/apache/activemq-artemis/pull/8
ARTEMIS-127 Adding activemq unit test module to Artemis
This test module brings in activemq unit tests and run them
against Artemis broker.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gaohoward/activemq-artemis artemis127_master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/8.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 #8
----
commit 9b89104339fae58ac55ba45a2a9bfd12f0954e38
Author: Howard Gao <hg...@redhat.com>
Date: 2015-05-29T07:43:33Z
ARTEMIS-127 Adding activemq unit test module to Artemis
This test module brings in activemq unit tests and run them
against Artemis broker.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107915237
Hi Clebert,
I got the PR build passed finally. Can you take a look at it?
Thanks
Howard
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107280602
@gaohoward Howard, just add @Deprecated on those classes.. that's all.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by thiagokronig <gi...@git.apache.org>.
Github user thiagokronig commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110427482
@clebertsuconic please update the parent version pom: <version>1.0.0-SNAPSHOT</version>
Should we have some kind of rule in the build to enforce that a child pom always conform with his parent, to at least generate a warning?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108700719
@clebertsuconic sure I'll.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107566978
@gaohoward you said you would send a new PR? no need.. just update this one...
let me know when you have done it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108151515
ok let's try that. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107281546
The code is only for compiling and running the tests to get the result. after that do a 'mvn clean' will get rid of all those codes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110558166
@thiagokronig where you are saying that?
and how to enforce that kind of rule. I'm not sure where it's missing a parent as snapshot
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107043394
@gaohoward It's an easy fix... and we are importing the code on moving forward.
If this is verbatim we could just use it straight from maven, but if you are importing/committing the code we should fix it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107305686
I've updated my last comment.. I meant, if you need the patch, we will need to refacdtor this somehow it's independent and you should have the code committed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108499190
@gaohoward you will probably need to tell Thiago on specifics where he could help you. It's hard to split a task like without duplicating/wasting work, without some proper communication.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-109167037
@clebertsuconic @thiagokronig can you take a look at this? I've gotten rid of most of the amq5 code.
do 'mvn -Pactivemq5-unit-tests' and you can kick of the test suite. Many tests failures but the point is can we make this as the first step and make it into the repo, so that tests can be fixed based on this framework?
Thanks
Howard
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107780140
I got something messed up. fixing it now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107951707
This doesn't seem ready to me. I don't see a reason for bringing the entire activemq5 broker as a source folder inside a test folder.
I will look for you on the IRC.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110425687
@gaohoward I merged a rebased version of your PR.. so please rebase your branch before continuing working on anything around this.
since this is not importing the whole activemq5 broker we can fix anything on moving forward now. Thanks Howard.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110423683
I'm ok with this change!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-109855586
@gtully
I tried and got some compilation error, looks like the wrapper is referencing some of the API in broker jar (like Broker, TransportConnector). Anyway I think it's a good idea if we can make the dependency simpler. I think we can optimized it later. If the framework is ok and I think we need to get it in the branch asap, so future works (fixing tests) can follow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by thiagokronig <gi...@git.apache.org>.
Github user thiagokronig commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108443642
Hi, Howard Gao, if you wish, we could work together on this.
On Wed, Jun 3, 2015 at 8:46 AM Howard Gao <no...@github.com> wrote:
> Hi Martyn, what you suggested makes sense for sure. Thanks.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/activemq-artemis/pull/8#issuecomment-108329879>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108702094
@thiagokronig Hi Thiago right now I'm trying to get rid of the activemq 5 source code, only have a few classes to wrap the Artemis broker for the test. There are some compilation errors I need to solve after removing the code. I'd like to know what's your thought about the approach we are doing and how we can improve it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by thiagokronig <gi...@git.apache.org>.
Github user thiagokronig commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108712726
I am all forward for reusing code/ideas for tests from ActiveMQ! We can accomplish that with an objective in mind: don't replicate/depend on ActiveMQ, just refactor the tests a lot to be as most agnostic as possible.
I will work on this friday.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-106826266
Please, rebase and fix those?
http://errorprone.info/bugpattern/DepAnn
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by thiagokronig <gi...@git.apache.org>.
Github user thiagokronig commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110814326
@clebertsuconic, here:
tests/activemq5-unit-tests/pom.xml:23:
<version>1.0.0-SNAPSHOT</version>
Changing to 1.0.1-SNAPHOT breaks the build because of ErrorProne.
I'm fixing those errors at this moment. Should submit a PR in the following
hours.
On Tue, Jun 9, 2015 at 11:07 PM clebertsuconic <no...@github.com>
wrote:
> @thiagokronig <https://github.com/thiagokronig> where you are saying
> that?
> and how to enforce that kind of rule. I'm not sure where it's missing a
> parent as snapshot
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/activemq-artemis/pull/8#issuecomment-110558166>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-106826140
Howard, your PR doesn't build because of these issues:
/work/apache/six-merge/artemis-selector/target/generated-sources/javacc/org/apache/activemq/artemis/selector/hyphenated/SimpleCharStream.java:223: error: [DepAnn] Deprecated item is not annotated with @Deprecated
public int getColumn() {
^
(see http://errorprone.info/bugpattern/DepAnn)
Did you mean '@Deprecated'?
/work/apache/six-merge/artemis-selector/target/generated-sources/javacc/org/apache/activemq/artemis/selector/hyphenated/SimpleCharStream.java:232: error: [DepAnn] Deprecated item is not annotated with @Deprecated
public int getLine() {
^
(see http://errorprone.info/bugpattern/DepAnn)
Did you mean '@Deprecated'?
/work/apache/six-merge/artemis-selector/target/generated-sources/javacc/org/apache/activemq/artemis/selector/strict/SimpleCharStream.java:223: error: [DepAnn] Deprecated item is not annotated with @Deprecated
public int getColumn() {
^
(see http://errorprone.info/bugpattern/DepAnn)
Did you mean '@Deprecated'?
/work/apache/six-merge/artemis-selector/target/generated-sources/javacc/org/apache/activemq/artemis/selector/strict/SimpleCharStream.java:232: error: [DepAnn] Deprecated item is not annotated with @Deprecated
public int getLine() {
^
(see http://errorprone.info/bugpattern/DepAnn)
Did you mean '@Deprecated'?
Note: Some input files use unchecked or unsafe operations.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107289228
Just commit the code you need. a patch is not a good solution on moving forward. It's impossible to maintain it.
If you don't need the patch, then we need to refactor the tests properly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] activemq-artemis pull request: ARTEMIS-127 Adding
activemq unit te...
Posted by chinxxx <xx...@gmail.com>.
Thanks, in advance, for any info.
ไฮไลท์บอล คู่เด็ด ทุกคู่ ทุกแมต ไม่พลาด ได้ที่นี่ ไฮไลท์ฟุตบอล
<https://cliphighlight.buaksib.com>
--
View this message in context: http://activemq.2283324.n4.nabble.com/GitHub-activemq-artemis-pull-request-ARTEMIS-127-Adding-activemq-unit-te-tp4697043p4697389.html
Sent from the ActiveMQ - Dev mailing list archive at Nabble.com.
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-109303356
I'm good either way. The good thing about having the tests committed is it would be easier to maintain them on the long term.
Howard had a whole broker copy on the commit here that's why we asked the change.
@gaohoward I'm taking a day off today, I can look on this on monday. You can keep working on this.
BTW: You had a fix as part of this PR. is it lost or did you squash it? don't forget to update JIRA please.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108464364
Hi Thiago Kronig, it'll be great if you can lend a hand. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107278310
Hi Clebert,
I don't have a good idea as to how to fix this. What the module does is actually download the activmq 5 source code (unit tests, activemq-broker, etc, see pom.xml) and patch some of the code (only a few, in order to run and test against the wrapped broker). None of the downloaded code are committed.
Maybe it's a good idea to exclude them from the check?
Howard
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by andytaylor <gi...@git.apache.org>.
Github user andytaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-106816951
most of the tests fail clebert so we shouldn't have it enabled by default i dont think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-106868119
last time I ran the test suite I got a little over 30% pass rate. So there still a long way to fix it. I don't think we should enable it by default for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-106879458
Please correct those as we have error-prone on the compilation.
Look at my last commit and the post to the activemq-dev-list
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/8
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107280665
@gaohoward we don't commit the code? why not actually commit the code and carry on?
let me talk to you tomorrow about this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108295200
Hi Howard. I realise that @clebertsuconic already mentioned this but it looks like you are importing a lot of code from ActiveMQ. Reuse of test suites is probably going to required for more than just OpenWire going forward. It would be great if you could find a clean solution for this.
We should try to avoid importing code from ActiveMQ into Artemis as it will just sit there and rot whilst the ActiveMQ code base moves forward. A neater way to do this would be to import the test module artifact and extend the relevant pieces. If this is not possible right now it is probably worth implementing some extension points in the ActiveMQ code base to allow reuse of their test suites.
A start might be to implement an Artemis broker service as @clebertsuconic suggested.
Let's have a chat on IRC.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107992912
There must be a better way. Why don't you just implement BrokerService properly instead of depending on the whole activemq5?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108699289
https://github.com/apache/activemq-artemis/pull/8
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by thiagokronig <gi...@git.apache.org>.
Github user thiagokronig commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110862182
yes I can.
On Wed, Jun 10, 2015 at 1:44 PM clebertsuconic <no...@github.com>
wrote:
> I think we should keep Error Prone on the compilation.. unless it starts
> to bother people.
> So far I think it's useful.
>
> can you send the PR?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/activemq-artemis/pull/8#issuecomment-110828541>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-106867647
Hi Clebert,
those @deprecated tags are from activemq 5 source code, we don't need to correct them imo.
This module just get them from amq repository verbatim, and are not part of our code.
Anyway I'm not aware we have such check, local build on my machine doesn't throw out those errors.
Howard
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107970469
I need activemq5 client and broker sources because I need to modify some of them in order to inject the Artemis broker into activemq5 broker service. I bring in the sources only for the few changes to compile and run the tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110555396
Thanks Clebert!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110828541
I think we should keep Error Prone on the compilation.. unless it starts to bother people.
So far I think it's useful.
can you send the PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108329879
Hi Martyn, what you suggested makes sense for sure. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-108721184
That'll be great. I'll update the branch soon and you can have a look. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by thiagokronig <gi...@git.apache.org>.
Github user thiagokronig commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-110823751
@clebertsuconic we are skipping checkstyle, may we skip Error Prone also,
just to maintain certain compatibility with the ActiveMQ codebase?
On Wed, Jun 10, 2015 at 1:09 PM Thiago Kronig <th...@gmail.com>
wrote:
> @clebertsuconic, here:
>
> tests/activemq5-unit-tests/pom.xml:23:
> <version>1.0.0-SNAPSHOT</version>
>
> Changing to 1.0.1-SNAPHOT breaks the build because of ErrorProne.
> I'm fixing those errors at this moment. Should submit a PR in the
> following hours.
>
>
> On Tue, Jun 9, 2015 at 11:07 PM clebertsuconic <no...@github.com>
> wrote:
>
>> @thiagokronig <https://github.com/thiagokronig> where you are saying
>> that?
>> and how to enforce that kind of rule. I'm not sure where it's missing a
>> parent as snapshot
>>
>> —
>> Reply to this email directly or view it on GitHub
>> <https://github.com/apache/activemq-artemis/pull/8#issuecomment-110558166>
>> .
>>
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107951267
@gaohoward I don't understand this... what's under ./activemq5-unit-tests... it seems the whole activemq5 broker implementation. You even have a ProtocolManagerFactory for the OpenWireProtocol there.
Why do you need a whole broker implementation for tests? you could just depend on the maven package.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gaohoward <gi...@git.apache.org>.
Github user gaohoward commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-107302365
ok, thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-127 Adding activemq unit te...
Posted by gtully <gi...@git.apache.org>.
Github user gtully commented on the pull request:
https://github.com/apache/activemq-artemis/pull/8#issuecomment-109288077
@gaohoward - this may be naive, but what happens if your pom dependency for 5.x is just:
```
<dependency>
<groupId>org.apache.activemq</groupId>
<artifactId>activemq-unit-tests</artifactId>
<version>...</version>
<type>test-jar</type>
<scope>test</scope>
<exclusions>
<!-- 5.x transative dependencies we implement to get the tests to run over artemis -->
<exclusion>
<groupId>org.apache.activemq</groupId>
<artifactId>activemq-broker</artifactId>
</exclusion>
<exclusions>
</dependency>
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---