You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "mattrpav (via GitHub)" <gi...@apache.org> on 2023/04/24 14:43:44 UTC
[GitHub] [activemq] mattrpav opened a new pull request, #1002: [AMQ-9239] Host joram jms unit tests ahead of converting to jakarta.jms
mattrpav opened a new pull request, #1002:
URL: https://github.com/apache/activemq/pull/1002
Currently this module is unmaintained and we need to convert it for jakarta broker support.
Modules that use this module:
activemq-amqp
activemq-unit-tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on a diff in pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #1002:
URL: https://github.com/apache/activemq/pull/1002#discussion_r1180493579
##########
activemq-tooling/activemq-joram-jms-tests/src/main/java/org/objectweb/jtests/jms/conform/connection/TopicConnectionTest.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * Copyright 2009 Red Hat, Inc.
Review Comment:
I noticed there is a Red Hat reference in some files like here, I assume we need to get rid of this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav merged pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav merged PR #1002:
URL: https://github.com/apache/activemq/pull/1002
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #1002:
URL: https://github.com/apache/activemq/pull/1002#issuecomment-1520433766
Personally I dont see enough value in these tests to copy+convert them, a lot of it is just testing basic stuff the rest of the tests should be covering themselves. I would simply drop their usage (and did exactly that elsewhere). They dont cover any of the newer stuff, not having been updated in over a decade. I cant remember a single time theyve actually shown a failure. The actual official spec tests are easily available now unlike back then if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1002:
URL: https://github.com/apache/activemq/pull/1002#issuecomment-1520973242
@gemmellr I think removing it would be a good end goal. However, right now its deeply integrated into amqp tests. I'll make a JIRA for removing joram and refactoring the amqp tests post-jakarta work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on a diff in pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #1002:
URL: https://github.com/apache/activemq/pull/1002#discussion_r1180504313
##########
activemq-tooling/activemq-joram-jms-tests/src/main/java/org/objectweb/jtests/jms/conform/connection/TopicConnectionTest.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * Copyright 2009 Red Hat, Inc.
Review Comment:
Already corrected in the jakarta PR commit:
https://github.com/apache/activemq/pull/996/commits/17d7f024873765ba05ac735f53dc321117dc8881
##########
activemq-tooling/activemq-joram-jms-tests/src/main/java/org/objectweb/jtests/jms/conform/connection/TopicConnectionTest.java:
##########
@@ -0,0 +1,171 @@
+/*
+ * Copyright 2009 Red Hat, Inc.
Review Comment:
Already corrected in the jakarta PR commit:
See: https://github.com/apache/activemq/pull/996/commits/17d7f024873765ba05ac735f53dc321117dc8881
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #1002:
URL: https://github.com/apache/activemq/pull/1002#issuecomment-1521762735
Knowing how the Joram tests work, 'deeply integrated' seems a bit of an exageration. Theres one package of classes that could just be outright deleted, and it looks like there are a couple of other tests misusing some related code to enable broker protocol tracing that arguably shouldnt be enabled anyway and can be got other ways. Seems trivial to remove.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] gemmellr commented on pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #1002:
URL: https://github.com/apache/activemq/pull/1002#issuecomment-1522214913
Just giving some feedback on a simpler approach, its not something I will be working on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1002:
URL: https://github.com/apache/activemq/pull/1002#issuecomment-1521856431
@gemmellr Yep, I've got AMQ-9249 tee'd up for after getting the Jakarta broker work in order.
If you've got spare cycles now, go for it! I'm happy to rebase from there, if you can get it merged in before the Jakarta work is done.
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #1002: [AMQ-9239] Host joram-jms-unit tests ahead of converting to jakarta.jms
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1002:
URL: https://github.com/apache/activemq/pull/1002#issuecomment-1522219546
@gemmellr no worries, appreciated!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org